[m-dev.] for review: --stack-trace-higher-order

[Fergus Henderson]

On 01-Nov-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> compiler/llds_out.m:
> 	Improve some predicate names, in an effort to prevent confusion
> 	about what a "file" is (since the code uses more than meaning,
> 	given the presence of --split-c-files).

s/more than meaning/more than one meaning/ ?

> runtime/mercury_goto.c:
> 	Simplify the conditions under which labels get added to the label
> 	table with:
> 
> 	- The macros init_{entry,label,local}_ai always add
> 	rhe label to the label table without a layout structure.

s/rhe/the/

> Index: compiler/continuation_info.m
>  	%
>  	% Takes the pred_proc_id, entry label, the number of stack slots,
> -	% the code model for this proc, and the stack slot of the succip
> -	% in this proc (if there is one).
> +	% the determinism of the proc, the stack slot of the succip
> +	% in this proc (if there is one), the label of the call event
> +	% if there is such an event), the slot containing the saved from-full

s/if/(if/

> +continuation_info__maybe_process_proc_llds(Instructions, PredProcId,
> +		ModuleInfo, ContInfo0, ContInfo) :-
> +	PredProcId = proc(PredId, ProcId),
> +	module_info_pred_proc_info(ModuleInfo, PredId, ProcId, PredInfo, _),

If you don't need the ProcInfo, then it's better to call
`module_info_pred_info' rather than `module_info_pred_proc_info'.

> +++ array.m	1998/10/30 05:27:37
> @@ -263,9 +263,15 @@
>  Define_extern_entry(mercury____Unify___array__array_1_0);
>  Define_extern_entry(mercury____Index___array__array_1_0);
>  Define_extern_entry(mercury____Compare___array__array_1_0);
> -MR_MAKE_STACK_LAYOUT_ENTRY(mercury____Unify___array__array_1_0);
> -MR_MAKE_STACK_LAYOUT_ENTRY(mercury____Index___array__array_1_0);
> -MR_MAKE_STACK_LAYOUT_ENTRY(mercury____Compare___array__array_1_0);
> +MR_MAKE_PROC_LAYOUT(mercury____Unify___array__array_1_0,
> +	MR_DETISM_SEMI, -1, MR_LVAL_TYPE_UNKNOWN,
> +	MR_PREDICATE, ""array"", ""unify_array"", 2, 0);
> +MR_MAKE_PROC_LAYOUT(mercury____Index___array__array_1_0,
> +	MR_DETISM_DET, -1, MR_LVAL_TYPE_UNKNOWN,
> +	MR_PREDICATE, ""array"", ""index_array"", 2, 0);
> +MR_MAKE_PROC_LAYOUT(mercury____Compare___array__array_1_0,
> +	MR_DETISM_DET, -1, MR_LVAL_TYPE_UNKNOWN,
> +	MR_PREDICATE, ""array"", ""compare_array"", 3, 0);

I think it would be nicer if there was a named macro there rather than
a magic number `-1'.

runtime/mercury_ho_call.c:
> -	int	type_arity;
> -
> -	type_arity = pop();
> -	MR_succip = (Code *) pop();
> +	MR_succip = (Code *) MR_stackvar(2);
>  	save_registers();
> -	r2 = virtual_reg(type_arity + 1);
> +	r2 = virtual_reg(MR_stackvar(1) + 1);
> +	decr_sp_pop_msg(2);

The use of the named variable `type_arity' is good
documentation, so I think it should stay.

> Index: runtime/mercury_stack_layout.h

There was no log message for the changes to this file.

> +** For the native garbage collector, we will to add meaningful live value
> +** information as well to these macros.

s/we will to add/we will need to add/ ?

> --- mercury_stack_trace.c	1998/10/30 04:51:07	1.20
> +++ mercury_stack_trace.c	1998/11/01 04:25:09
> @@ -194,7 +194,7 @@
>  
>  	label = MR_lookup_internal_by_addr(success);
>  	if (label == NULL) {
> -		*problem_ptr = "reached label with no stack trace info";
> +		*problem_ptr = "reached unregistered label";
>  		return STEP_ERROR_AFTER;
>  	}
>  

I think the original message will be easier for users to understand.
What should users know about label registration?

You could perhaps make it "reached label for which stack trace information
is not available".

Apart from that, it looks OK, but I haven't finished reviewing the
changes to llds_out.m yet.

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.