[m-dev.] for review: Declarative debugger front end

[Fergus Henderson]

On 09-Apr-1999, Mark Anthony BROWN <dougl at cs.mu.OZ.AU> wrote:
> browser/declarative_debugger:

s/:/.m:/

> +#ifdef DEBUG_DD_BACK_END

All new configuration macros should (a) start with `MR_'
and (b) be documented in runtime/mercury_conf_param.h.

> +extern void
> +MR_edt_root_node(Word EDT, Word *Node)
> +{
> +	MR_Edt_Node		*edt;
> +	MR_Stack_Layout_Entry	*entry;
> +	ConstString		name;
> +	Word			args;
>  
> -	if (root->MR_edt_node_tag == MR_EDT_WRONG_ANSWER_IMPLICIT) {
> -		for (i = 0; i < level + 1; i++) {
> -			printf("    ");
> -		}
> -		printf("/* implicit */\n");
> +	edt = (MR_Edt_Node *) EDT;
> +	entry = edt->MR_edt_node_layout->MR_sll_entry;
> +	
> +	switch (edt->MR_edt_node_tag) {
> +		case MR_EDT_WRONG_ANSWER_EXPLICIT:
> +		case MR_EDT_WRONG_ANSWER_IMPLICIT:
> +			name = MR_edt_root_node_name(entry);
> +			args = MR_edt_root_node_args(edt);
> +			incr_hp(*Node, 2);
> +			field(mktag(0), *Node, 0) = (Word) name;
> +			field(mktag(0), *Node, 1) = args;
> +			break;

Here you're using incr_hp() inside a C function,
without restoring it.  You should probably be using
either incr_saved_hp() or MR_TRACE_USE_HP().

> +static Word
> +MR_edt_root_node_args(const MR_Edt_Node *edt)
>  {
> +	int			i;
> +	int			argc;
> +	Word			arglist;
> +	Word			tail;
> +	Word			head;
> +
> +	argc = edt->MR_edt_node_layout->MR_sll_var_count;
> +
> +	arglist = list_empty();
> +	for (i = argc - 1; i >= 0; i--) {
> +		tail = arglist;
> +		incr_hp(head, 2);
> +		field(mktag(0), head, UNIV_OFFSET_FOR_TYPEINFO) =
> +			edt->MR_edt_node_arg_types[i];
> +		field(mktag(0), head, UNIV_OFFSET_FOR_DATA) =
> +			edt->MR_edt_node_arg_values[i];
> +		arglist = list_cons(head, tail);

Likewise here.

declarative_debugger.m:
> :- type edt_node
> 	--->	wrong_answer(string, list(univ)).

You should document the meaning of those two fields.

> :- pred edt_children_1(evaluation_tree, list(evaluation_tree)).
> :- mode edt_children_1(in, out) is det.
> 
> edt_children_1(Child, Siblings) :-
> 	(
> 		edt_sibling(Child, Sibling)
> 	->
> 		edt_children_1(Sibling, Siblings0),
> 		Siblings = [Sibling | Siblings0]
> 	;
> 		Siblings = []
> 	).

s/_1/_2/g  (for consistency with the naming conventions used elsewhere).

> :- pred analyse_edt_1(evaluation_tree, declarative_bug, io__state, io__state).
> :- mode analyse_edt_1(in, out, di, uo) is det.
> 
> analyse_edt_1(EDT, Bug) -->
> 	{ edt_children(EDT, Children) },
> 	analyse_children(Children, wrong(EDT), Bug).

Likewise.

Otherwise, that change looks fine.

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