[m-rev.] for review: delete trace level decl

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Sep 27 13:39:44 AEST 2002


On 27-Sep-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> doc/user_guide.texi:
> 	Delete or comment out all mentions of --trace decl and decldebug
> 	grades.

It would be good to have an XXX comment there somewhere explaining
why the commented-out stuff was there (rather than being deleted),
and why it was commented out.

> +++ compiler/layout.m	26 Sep 2002 23:44:29 -0000
> @@ -56,7 +56,8 @@
>  			string_table		:: string,
>  			proc_layout_names	:: list(layout_name),
>  			file_layouts		:: list(file_layout_data),
> -			trace_level		:: trace_level
> +			trace_level		:: trace_level,
> +			suppressed_events	:: int

There should be a comment documenting this field (e.g. the one from
the log message).

It might be nicer to use an equivalence type or a no-tag type, e.g. named
`suppressed_events' or `encoded_trace_suppress_items', rather than using `int'
directly.  In that case, the comment could go on the definition of the type.

>  :- pred output_module_layout_data_defn(module_name::in, int::in,
>  	string::in, list(layout_name)::in, list(file_layout_data)::in,
> -	trace_level::in, decl_set::in, decl_set::out,
> +	trace_level::in, int::in, decl_set::in, decl_set::out,
>  	io__state::di, io__state::uo) is det.

Here it would be nicer to use the type mentioned above.

> Index: compiler/trace_params.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/trace_params.m,v
> retrieving revision 1.12
> diff -u -b -r1.12 trace_params.m
> --- compiler/trace_params.m	9 Sep 2002 08:54:10 -0000	1.12
> +++ compiler/trace_params.m	27 Sep 2002 01:26:41 -0000
> @@ -92,15 +92,16 @@
>  	% This is used to represent the trace level in the module layout.
>  :- func trace_level_rep(trace_level) = string.
>  
> +:- func encode_suppressed_events(trace_suppress_items) = int.

Likewise here.

> +encode_suppressed_events(SuppressedEventSet) = SuppressedEventsInt :-
> +	set__to_sorted_list(SuppressedEventSet, SuppressedEventList),
> +	list__foldl(maybe_add_suppressed_event, SuppressedEventList,
> +		0, SuppressedEventsInt).

It would be nicer to just use set__fold.

> Index: doc/user_guide.texi
> @@ -1640,27 +1644,20 @@
>  @item deep
>  A procedure compiled with trace level @samp{deep}
>  will always generate all the events requested by the user.
> -By default, this is all the usual events,
> +By default, this is all possible usual events,

For me, that wording seems rather awkward.
How about just "By default, this is all possible events,"?


Otherwise that change looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list