[m-dev.] for review: a big step towards the trace-based debugger (part 2 of 3)

[Fergus Henderson]

On 20-Mar-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
...
> +	%
> +	% In addition, a typeclass-info may be non-local to a goal if
> +	% any of the non-local variables for that goal are
> +	% polymorphically typed and are constrained by the typeclass
> +	% constraints for that typeclass-info variable
> +	%
> +:- pred goal_util__extra_nonlocal_typeinfos(map(var, type_info_locn),
> +		map(var, type), hlds_goal, set(var)).
> +:- mode goal_util__extra_nonlocal_typeinfos(in, in, in, out) is det.

Missing full stop at the end of the comment.

> +% The number of type parameters is never stored as it is not needed --
> +% the type parameter vector will simply be indexed by the type variable's
> +% variable number stored within pseudo-typeinfos inside the elements
> +% of the live data pairs vectors. Since we allocate type variable numbers
> +% sequentially, the type parameter vector will usually be dense. However,
> +% in some cases, XXX

Unfinished comment.

> Index: compiler/termination.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/termination.m,v
> retrieving revision 1.11
> diff -u -r1.11 termination.m
> --- termination.m	1998/03/18 08:07:47	1.11
> +++ termination.m	1998/03/19 06:27:21
> @@ -47,8 +47,8 @@
>  
>  :- interface.
>  
> -:- import_module io, bool, std_util, list.
>  :- import_module prog_data, hlds_module, hlds_pred, term_util.
> +:- import_module list, io, bool, std_util.
>  
>  	% Perform termination analysis on the module.
>  
> @@ -84,7 +84,7 @@
>  :- import_module mercury_to_mercury, varset, type_util, special_pred.
>  :- import_module modules.
>  
> -:- import_module map, int, char, string, relation.
> +:- import_module list, map, int, char, string, relation.
>  :- import_module require, bag, set, term.

I don't think there's any need to include `list' in both the
interface and implementation imports.  Just the interface import
should be enough.

>  :- type trace_port	--->	call
>  			;	exit
>  			;	fail
> -			;	ite_then(goal_path)
> -			;	ite_else(goal_path)
> -			;	switch(goal_path)
> -			;	disj(goal_path).
> +			;	ite_then(goal_path, set(var))
> +			;	ite_else(goal_path, set(var))
> +			;	switch(goal_path, set(var))
> +			;	disj(goal_path, set(var)).

You should say what the set(var)s here represent.

> +	% Generate code for a trace event, returning the
> +:- pred trace__generate_event_code(trace_port::in, trace_info::in,

Unfinished comment.

> +trace__fail_vars(ProcInfo, FailVars) :-
> +	proc_info_headvars(ProcInfo, HeadVars),
> +	proc_info_arg_info(ProcInfo, ArgInfos),
> +	assoc_list__from_corresponding_lists(HeadVars, ArgInfos, Args),
> +	arg_info__build_input_arg_list(Args, ArgList),
> +	assoc_list__keys(ArgList, InputArgs),
> +		% We do not yet delete input vars that have any components
> +		% that could be clobbered, because the modules of the mode
> +		% system do not have any utility predicates for testing for
> +		% this.
> +	set__list_to_set(InputArgs, FailVars).

The comment should have an "XXX".

It would be better to at least delete the ones that are fully clobbered.
In fact, now that I think about it, those are the only ones you should
delete; for the others, the tracer should display the non-clobbered parts
(of course, it needs the inst for that, and currently we don't pass the
inst, but there's already an XXX for that, I think.)

> +:- pred trace__apply_pre_deaths(trace_port::in, list(var)::in, list(var)::out)
> +	is det.
> +
> +trace__apply_pre_deaths(call, LiveVars, LiveVars).
> +trace__apply_pre_deaths(exit, LiveVars, LiveVars).
> +trace__apply_pre_deaths(fail, LiveVars, LiveVars).
> +trace__apply_pre_deaths(ite_then(_, PreDeaths), LiveVars0, LiveVars) :-
> +	set__list_to_set(LiveVars0, LiveVars0Set),
> +	set__difference(LiveVars0Set, PreDeaths, LiveVarsSet),
> +	set__to_sorted_list(LiveVarsSet, LiveVars).
> +trace__apply_pre_deaths(ite_else(_, PreDeaths), LiveVars0, LiveVars) :-
> +	set__list_to_set(LiveVars0, LiveVars0Set),
> +	set__difference(LiveVars0Set, PreDeaths, LiveVarsSet),
> +	set__to_sorted_list(LiveVarsSet, LiveVars).
> +trace__apply_pre_deaths(switch(_, PreDeaths), LiveVars0, LiveVars) :-
> +	set__list_to_set(LiveVars0, LiveVars0Set),
> +	set__difference(LiveVars0Set, PreDeaths, LiveVarsSet),
> +	set__to_sorted_list(LiveVarsSet, LiveVars).
> +trace__apply_pre_deaths(disj(_, PreDeaths), LiveVars0, LiveVars) :-
> +	set__list_to_set(LiveVars0, LiveVars0Set),
> +	set__difference(LiveVars0Set, PreDeaths, LiveVarsSet),
> +	set__to_sorted_list(LiveVarsSet, LiveVars).

Hmm, some code duplication here...

Why not use list__delete_all instead of the three set__ calls?

> Index: runtime/mercury_misc.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_misc.c,v
> retrieving revision 1.4
> diff -u -r1.4 mercury_misc.c
> --- mercury_misc.c	1998/03/16 12:23:33	1.4
> +++ mercury_misc.c	1998/03/20 08:06:01
> @@ -468,6 +468,7 @@
>  void 
>  fatal_error(const char *message) {
>  	fprintf(stderr, "Mercury runtime: %s\n", message);
> +	MR_trace_report();
>  	exit(1);
>  }

What about fatal_abort()?
It would be worth modifying that too.
Of course you probably need a different version that
outputs using write() instead of fwrite().

> --- mercury_stack_layout.h	1998/03/11 22:07:30	1.2
> +++ mercury_stack_layout.h	1998/03/18 05:05:54
> @@ -49,7 +53,8 @@
>  
>  #define MR_DETISM_FIRST_SOLN(d)		(((d) & 8) != 0)
>  
> -#define MR_DETISM_DET_CODE_MODEL(d)	(((d) & 1) == 0)
> +#define MR_DETISM_DET_CODE_MODEL(d)	(!MR_DETISM_AT_MOST_MANY(d) \
> +					|| MR_DETISM_FIRST_SOLN(d))

What about model_semi code?

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.