[m-dev.] for review: declarative debugger back end

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Dec 11 16:50:05 AEDT 1998


On 10-Dec-1998, Mark Anthony BROWN <dougl at cs.mu.OZ.AU> wrote:
> Fergus, would you like to review this please?

Sure.

> trace/mercury_trace_internal.{c,h}:
> 	Declare a global variable, MR_trace_decl_mode,  that stores the

s/,  /, /
   ^^  ^

> bool
> MR_trace_start_wrong_answer(MR_Trace_Cmd_Info *cmd, 

You should document the meaning of the `bool' return value.

> struct MR_Edt_Node_Struct {
> 						/*
> 						** Type of EDT node.
> 						*/
> 	MR_Edt_Node_Type		MR_edt_node_tag;
> 						/*
> 						** The layout of the EXIT port.
> 						*/
> 	const MR_Stack_Layout_Label	*MR_edt_node_layout;
> 						/*
> 						** The arguments.
> 						*/
> 	Word				*MR_edt_node_arg_values;
> 	Word				*MR_edt_node_arg_types;
> 	const char			*MR_edt_node_path;
> 						/*
> 						** The event numbers of the
> 						** CALL and EXIT events for
> 						** this proof.
> 						*/
> 	int				MR_edt_node_start_event;
> 	int				MR_edt_node_end_event;
> 						/*
> 						** The sequence number of the
> 						** CALL and EXIT events.
> 						*/
> 	int				MR_edt_node_seqno;
> 						/*
> 						** The rightmost child of this
> 						** node, or NULL if there are
> 						** no children.
> 						*/
> 	MR_Edt_Node			*MR_edt_node_children;
> 						/*
> 						** The next sibling to the
> 						** left of this node, or NULL
> 						** if this is the leftmost.
> 						*/
> 	MR_Edt_Node			*MR_edt_node_sibling;
> };

Hmm, the layout there makes that rather difficult to read, IMHO.

How about unindenting the comments?
e.g.

    struct MR_Edt_Node_Struct {
	    /*
	    ** Type of EDT node.
	    */
	    MR_Edt_Node_Type		MR_edt_node_tag;

	    ...


Also, I noticed that there were a lot of functions which take
a lot of arguments, many of which are the same (cmd, layout,
port, seqno, depth, ...).  It might be better to package
those arguments up into a struct.

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.



More information about the developers mailing list