[m-rev.] for review by Mark: synthesized attributes

[Mark Brown]

On 11-Dec-2006, Zoltan Somogyi <zs at csse.unimelb.edu.au> wrote:
> Implement synthesized attributes at user events.
> 
> doc/user_guide.texi:
> 	Document synthesized attributes.
> 
> runtime/mercury_stack_layout.h:
> 	Include information about synthesized attributes in layout structures.
> 
> 	Move the the information about user events that is not specific to a

s/the the/the/

> doc/user_guide.texi:
> 	Document synthesized attributes.

The log message already mentions this change.

> trace/mercury_trace_cmd_internal.c

That should be "trace/mercury_trace_internal.c:".

> trace/mercury_trace_cmd_browsing.[ch]:
> 	Pass the current setting of the user_event_context mdb command to
> 	runtime/mercury_stack_trace.c when printing the context of an event.

This was committed earlier, wasn't it?

> Index: compiler/hlds_out.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/hlds_out.m,v
> retrieving revision 1.413
> diff -u -b -r1.413 hlds_out.m
> --- compiler/hlds_out.m	5 Dec 2006 03:50:51 -0000	1.413
> +++ compiler/hlds_out.m	8 Dec 2006 15:16:31 -0000
> @@ -1777,15 +1777,16 @@
>          globals.io_lookup_string_option(dump_hlds_options, Verbose, !IO),
>          ( string.contains_char(Verbose, 'l') ->
>              write_indent(Indent, !IO),
> -            io.write_string("% event call\n", !IO)
> +            io.write_string("% event call\n", !IO),
> +            write_indent(Indent, !IO),
> +            io.write_string("event ", !IO)
>          ;
> -            true
> +            write_indent(Indent, !IO)
>          ),

"event " should always be written here.

> Index: runtime/mercury_stack_layout.h
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_stack_layout.h,v
> retrieving revision 1.104
> diff -u -b -r1.104 mercury_stack_layout.h
> --- runtime/mercury_stack_layout.h	5 Dec 2006 03:51:13 -0000	1.104
> +++ runtime/mercury_stack_layout.h	8 Dec 2006 09:07:36 -0000

> +/*
> +** The fields of MR_UserEventSpec:
>  **
> -** The num_attributes field gives the number of attributes. Once the runtime
> -** system knows the number of attributes of each kind of event, this field may
> -** disappear.
> +** The event_number field contains the name of the event.

s/event_number/event_name/

>  **
> -** The next three fields all point to an array whose length is the number of
> +** The num_attrs field gives the number of attributes.
> +**
> +** The next three fields all point to arrays whose length is the number of
>  ** attributes.
>  **
> -** MR_ue_attr_locns[i] gives the location where we can find the value of the
> -** (i+1)th attribute (since we start counting attributes at one).
> +** attr_names[i] gives the location where we can find the value of the
> +** i'th attribute.

Wrong description.

>  **
> -** MR_ue_attr_types[i] is the typeinfo giving the type of the (i+1)th
> -** attribute.
> +** attr_types[i] is the typeinfo giving the type of the i'th attribute.
>  **
> -** MR_ue_attr_names[i] gives the name of the (i+1)th attribute.
> -** (In the future, this field may disappear.)
> +** If the i'th attribute is synthesized, synth_attrs[i] points to the
> +** information required to synthesize it: the number of the attribute
> +** containing the synthesis function, and the list of the attributes
> +** that are the arguments of the synthesis function (represented as a length
> +** and an array).

I'd write that as "the number of the attribute containing the synthesis
function, the number of arguments of the synthesis function, and the array
of attribute numbers that are to act as those arguments" or something like
that.

> If the i'th attribute is not synthesized,
> +** synth_attrs[i] will be NULL.
> +** 
> +** The synth_attr_order field points to an array of attribute numbers that
> +** gives the order in which the values of the synthesized attributes should be
> +** evaluated. The array is ended by -1 as a sentinel.

Are the synthesized attributes always going to be calculated all at once?
If not, it might be better to represent the dependency graph rather than an
ordering, so that the number of attributes synthesized can be minimized.

>  ** 
> -** user_event->MR_ue_attr_var_nums[i] gives the variable number of the (i+1)th
> -** attribute. This field is used by the debugger to display the associated
> -** value just once (not twice, as both attribute and variable value) with
> -** "print *". (Note that We don't delete the variables that are also attributes
> -** from the set of live variables in layout structures, because that would
> -** require any native garbage collector to look at the list of attributes
> -** as well as the list of other variables, slowing it down.)
> +** The synth_attrs and synth_attr_order fields will both be NULL for events
> +** that have no synthesized attributes.
>  */
>  
> -struct MR_UserEvent_Struct {
> -	MR_uint_least16_t		MR_ue_port_number;
> -	const char			*MR_ue_port_name;
> -	MR_uint_least16_t		MR_ue_num_attrs;
> -	MR_LongLval			*MR_ue_attr_locns;
> -	MR_TypeInfo			*MR_ue_attr_types;
> -	const char			**MR_ue_attr_names;
> -	const MR_uint_least16_t		*MR_ue_attr_var_nums;
> +struct MR_SynthAttr_Struct {
> +	MR_int_least16_t		MR_sa_func_attr;
> +	MR_int_least16_t		MR_sa_num_arg_attrs;
> +	MR_uint_least16_t		*MR_sa_arg_attrs;

Those two fields have their type swapped: the number of attributes should
be unsigned and the attribute numbers should be signed.

> Index: tests/invalid/syntax_error_event.err_exp
> ===================================================================
> RCS file: tests/invalid/syntax_error_event.err_exp
> diff -N tests/invalid/syntax_error_event.err_exp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ tests/invalid/syntax_error_event.err_exp	9 Dec 2006 06:47:48 -0000
> @@ -0,0 +1,2 @@
> +could not parse syntax_error_event_spec
> +no input file:7: syntax error at symbol `/'

This file needs updating.

> Index: trace/mercury_trace_vars.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/trace/mercury_trace_vars.c,v
> retrieving revision 1.73
> diff -u -b -r1.73 mercury_trace_vars.c
> --- trace/mercury_trace_vars.c	6 Dec 2006 03:45:03 -0000	1.73
> +++ trace/mercury_trace_vars.c	9 Dec 2006 02:33:27 -0000
> @@ -60,8 +68,14 @@
>  ** variable numbers occurring in the RTTI are renumbered to be a dense set,
>  ** whereas the original variable numbers are not guaranteed to be dense.)
>  **
> -** The last two fields contain the value of the variable and the typeinfo
> -** describing the type of this value.
> +** For attribute value, the num field gives the position of this attribute

s/For attribute value/For all attributes/

Aside from that, this change looks fine.

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
--------------------------------------------------------------------------