[m-rev.] for review: atoms in the debugger

[Fergus Henderson]

On 31-Dec-2001, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Give the debugger the ability to print atoms.
> 
> +++ NEWS	2001/12/30 17:53:13
> @@ -155,6 +155,8 @@
>  * You can now navigate terms in the debugger by argument name as well as by
>    argument number.
>    
> +* You can now print atoms in the debugger.

This is an instance of the terminology issue.
Whatever terminology we end up using,
the NEWS file should explain the new feature in more detail.

> Index: browser/browser_info.m
...
> +:- pred deconstruct_browser_term(browser_term::in,
> +	string::out, int::out, list(univ)::out, bool::out) is det.
> +
> +:- pred limited_deconstruct_browser_term(browser_term::in, int::in,
> +	string::out, int::out, list(univ)::out, bool::out) is semidet.
> +
> +:- pred functor_browser_term(browser_term::in, string::out, int::out,
> +	bool::out) is det.

There should be comments in the interface saying what these procedures do.

> --- browser/sized_pretty.m	2001/06/26 06:16:22	1.4
...
> +:- pred sized_pretty__browser_term_to_string_line(browser_term::in,
> +	int::in, int::in, string::out) is det.

Likewise.

> +char_count_split(BrowserTerm, Params, char_count(Limit), Arity, Check, 
>  		char_count(FunctorSize), MaybeArgLimit, char_count(Limit),
>  		Params) :-
> -
> -	deconstruct(univ_value(Univ), Functor, _, Args),
> +	deconstruct_browser_term(BrowserTerm, Functor, _, Args, IsFunc),
>  	( Check = yes ->
>  		get_arg_length(Args, TotalLength, _)
>  	;
>  		TotalLength = 0
> +	),
> +	(
> +		IsFunc = yes,
> +		% Arity-2 times the string ", ", once "()", and once " = "
> +		FunctorSize = string__length(Functor) + 1 + 2 * (Arity)
> +	;
> +		IsFunc = no,
> +		% Arity-1 times the string ", ", and once "()"
> +		FunctorSize = string__length(Functor) + 2 * (Arity)

s/(Arity)/Arity/

Does this code occur in multiple places?

> +			% If an ordinary term is not deconstructed, it is
> +			% printed as "functor/Arity". If a synthetic function
> +			% term is not deconstructed, it is printed as
> +			% "functor/n+1", where n is Arity-1.
> +			% XXX we should

The XXX comment here is unfinished.

> Index: browser/util.m
>  
> +	% For use in representing unbound head variables in the "print atom"
> +	% commands in the debugger.
> +:- type unbound ---> '_'.

Another occurrence of the terminology issue.

> +++ runtime/mercury_type_info.h	2001/12/31 05:05:42
> @@ -301,6 +301,33 @@
>  #define MR_fill_in_tuple_type_info(arena, type_ctor_info, arity, vector) \
>      MR_fill_in_higher_order_type_info(arena, type_ctor_info, arity, vector)
>  
> +#define MR_static_type_info_arity_0(name, ctor)				\
> +	struct {							\
> +		MR_TypeCtorInfo field1;					\
> +	} name = {							\
> +		(MR_TypeCtorInfo) (ctor)				\
> +	};
> +
> +#define MR_static_type_info_arity_1(name, ctor, ti1)			\
> +	struct {							\
> +		MR_TypeCtorInfo field1;					\
> +		MR_TypeInfo 	field2;					\
> +	} name = {							\
> +		(MR_TypeCtorInfo) (ctor),				\
> +		(MR_TypeInfo)     (ti1)					\
> +	};

Why are the casts needed here?

> @@ -308,6 +335,9 @@
>  
>  #ifdef MR_HIGHLEVEL_CODE
>  
> +#define MR_builtin_type_ctor_info_name(TYPE, ARITY)			      \

s/#/  #/

> +++ trace/mercury_trace_browse.c	2001/12/30 16:04:39
> @@ -1052,6 +1080,10 @@
>  				problem = MR_trace_browse_exception(event_info,
>  					MR_trace_browse_internal,
>  					MR_BROWSE_CALLER_PRINT, format);
> +			} else if (streq(words[1], "atom")) {
> +				problem = MR_trace_browse_one_atom(MR_mdb_out,
> +					MR_trace_browse_atom_internal,
> +					MR_BROWSE_CALLER_PRINT, format);
>  			} else {
>  				problem = MR_trace_parse_browse_one(MR_mdb_out,
>  					words[1], MR_trace_browse_internal,
> @@ -1075,7 +1107,11 @@
>  		} else if (word_count == 2) {
>  			const char	*problem;
>  
> -			if (streq(words[1], "exception")) {
> +			if (streq(words[1], "atom")) {
> +				problem = MR_trace_browse_one_atom(MR_mdb_out,
> +					MR_trace_browse_atom_internal,
> +					MR_BROWSE_CALLER_BROWSE, format);
> +			} else if (streq(words[1], "exception")) {
>  				problem = MR_trace_browse_exception(event_info,
>  					MR_trace_browse_internal,
>  					MR_BROWSE_CALLER_BROWSE, format);

It would be slightly nicer to check for "atom" (or whatever name we decide to
use) and "exception" in the same order in these two places.

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