[m-rev.] for review: user-defined events

[Mark Brown]

Mostly minor details...

On 20-Nov-2006, Zoltan Somogyi <zs at csse.unimelb.edu.au> wrote:
> +read_event_specs(SpecsFileName, EventSpecMap, ErrorSpecs, !IO) :-
> +    % Currently, we convert the event specification file into a Mercury term
> +    % by using the yacc parser in the trace directory to create a C data
> +    % structure to represent its contents, writing out that data structure
> +    % as a Mercury term to a file (TermFileName), and then reading in the term
> +    % from that file.
> +    %
> +    % This is clumsy approach, since it requires access to the C code in the

s/is clumsy/is a clumsy/

> +:- pragma foreign_proc("C",
> +    read_specs_file(SpecsFileName::in, TermFileName::in, Problem::out,
> +    _IO0::di, _IO::uo),
> +    [will_not_call_mercury, promise_pure, tabled_for_io, thread_safe],
> +"
> +    int     spec_fd;
> +    FILE    *term_fp;
> +
> +    /*
> +    ** Using snprint instead of sprintf below would be very slightly safer,
> +    ** but unfortunately enabling the declaration of snprintf in the system's
> +    ** header files requires finding the right #defines, and this set of
> +    ** #defines is very system-dependent. Even having MR_HAVE_SNPRINTF set
> +    ** is no guarantee that calling snprintf won't lead to a warning from
> +    ** the C compiler.
> +    **
> +    ** There race conditions between opening the file, stat'ing the file

s/There/There are/

> +build_dep_map(_, _, _, _, [], !AttrTypeMap, !DepRel, !ErrorSpecs).
> +build_dep_map(EventName, FileName, LineNumber, KeyMap,
> +        [AttrTerm | AttrTerms], !AttrTypeMap, !DepRel, !ErrorSpecs) :-
> +    AttrTerm = event_attr_term(AttrName, AttrTypeTerm),
> +    bimap.lookup(KeyMap, AttrName, AttrKey),
> +    (
> +        AttrTypeTerm = event_attr_type_synthesized(_TypeTerm, SynthCall),
> +        SynthCall = event_attr_synth_call(FuncAttrName, ArgAttrs),
> +        record_arg_dependencies(EventName, FileName, LineNumber, KeyMap,
> +            AttrName, AttrKey, ArgAttrs, !DepRel, [], AttrErrorSpecs),
> +        (
> +            AttrErrorSpecs = [_ | _],
> +            !:ErrorSpecs = AttrErrorSpecs ++ !.ErrorSpecs
> +        ;
> +            AttrErrorSpecs = [],
> +            map.lookup(!.AttrTypeMap, AttrName, AttrType),
> +            ArgTypes = list.map(map.lookup(!.AttrTypeMap), ArgAttrs),

I think this will throw an exception if the user tries to use a higher
order attribute (i.e., if they have a synthesized argument that applies
one function attribute to another).  See next comment.

> +:- func convert_term_to_type(event_attr_type_term) = mer_type.
> +
> +convert_term_to_type(Term) = Type :-
> +    Term = event_attr_type_term(Name, Args),
> +    (
> +        Args = [],
> +        builtin_type_to_string(BuiltinType, Name)
> +    ->
> +        Type = builtin_type(BuiltinType)
> +    ;
> +        SymName = string_to_sym_name(Name),
> +        ArgTypes = list.map(convert_term_to_type, Args),
> +        Type = defined_type(SymName, ArgTypes, kind_star)
> +    ).

What exactly are the types allowed for attributes?  Did you mean to exclude
tuples?  If so, you should document that in the user manual.  Also document
that attributes cannot be polymorphic, and when synthesized attributes are
documented you should also mention that function attributes cannot be used
as arguments to other functions.

> + at c XXX replace with more realistic example
> + at example
> +	event nodiag_fail(
> +		test_failed:	string,
> +		arg_b:		int,
> +		arg_d:		int,
> +		arg_list:	list(int)
> +	)
> +
> +	event safe_test(
> +		test_list:	listint

s/listint/list(int)/

or define that type synonym here.  It would be worth specifying exactly
which types are visible in this file.

> Index: trace/mercury_event_spec.c
> ===================================================================
> RCS file: trace/mercury_event_spec.c
> diff -N trace/mercury_event_spec.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ trace/mercury_event_spec.c	19 Nov 2006 14:55:02 -0000
> @@ -0,0 +1,236 @@
> +/*
> +** vim: ts=4 sw=4 expandtab
> +*/
> +/*
> +** Copyright (C) 1998-2002, 2005-2006 The University of Melbourne.

The years here are a bit off.

Finally, it would be good if there were some test cases in tests/invalid
to check the various errors that can arise in event spec files.

Cheers,
Mark.

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------