[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