[m-dev.] for review: generating traces

[Fergus Henderson]

Zoltan Somogyi, you wrote:
> 
> First implementation of the generation of traces for Opium-style trace
> analysis.
...

> +	(
> +		{ CodeModel = model_semi },
> +			% check whether the call is to a nondet procedure
> +			% for which are pruning all solutions after the first
> +		{ module_info_preds(ModuleInfo, Preds) },
> +		{ map__lookup(Preds, PredId, PredInfo) },
> +		{ pred_info_procedures(PredInfo, Procs) },
> +		{ map__lookup(Procs, ModeId, ProcInfo) },
> +		{ proc_info_interface_code_model(ProcInfo, InterfaceModel) },
> +		{ InterfaceModel = model_non }
> +	->
> +		{ EffectiveModel = model_non }
> +	;
> +		{ EffectiveModel = CodeModel }
> +	),

What about det procedures calling multidet ones?

> @@ -350,18 +380,26 @@
>  		{ CodeModel \= model_non }
>  	->
>  		{ SuccipSlot is MainSlots + 1 },
> -		{ SaveSuccipCode = node([
> +		{ SaveCode = node([
>  			assign(stackvar(SuccipSlot), lval(succip)) -
>  				"Save the success ip"
>  		]) },

Hmm, `SaveSuccipCode' seems to me to be a better variable name.
Any reason for this change?

> +:- pred code_info__get_proc_code_model(code_model, code_info, code_info).
> +:- mode code_info__get_proc_code_model(out, in, out) is det.

There is already a predicate code_info__get_proc_model
which does that.

> :- type trace_info	--->	trace_info(
> 					lval,	% stack slot of call seq num
> 					lval	% stack slot of call depth
> 				).

I would prefer the layout to be like this:

:- type trace_info
	--->	trace_info(
			lval,	% stack slot of call seq num
			lval	% stack slot of call depth
		).

(as recommended in our coding standard).
Then you might even have room to spell out "seq" ;-)
(which would make it easier for non-native English speakers).

> int	mr_trace_call_seq = 0;
> int	mr_trace_call_depth = 0;

s/mr_/MR_/g

> /*
> ** mercury_trace.h - defines the interface between
> ** the tracing subsystem and compiled code.
> */
> 
> #ifndef MERCURY_TRACE_H
> #define MERCURY_TRACE_H
> 
> typedef	enum {
> 	CALL, EXIT, FAIL
> } mr_trace_port;

s/mr_/MR_/g
s/CALL/MR_CALL/
s/EXIT/MR_EXIT/
s/FAIL/MR_FAIL/

> typedef	enum {
> 	DET, SEMI, NON
> } mr_trace_code_model;

Please insert MR_ prefixes.

> #define	trace_incr_seq()	++mr_trace_call_seq
> #define	trace_incr_depth()	++mr_trace_call_depth
> #define	trace_reset_depth(d)	mr_trace_call_depth = d

Please insert MR_ prefixes.
It would be a good idea to put some parentheses around
the bodies of the macros definitions.

> extern	void	trace(

Please insert an MR_ prefix.

I'd like to see the diff for code_info.m
and the new files again when you have addressed the above issues.

Cheers,
	Fergus.

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