[m-dev.] for review: procedure bodies for the declarative debugger

[Zoltan Somogyi]

On 25-Sep-2000, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> One thing to bear in mind with this is that it might be nice if we
> could provide some equivalent to Prolog's `clause/1', e.g. for writing
> certain kinds of meta-interpreters.  The infrastructure here looks
> like it could be quite useful for that.

Not without extra work. For example, when the code constructs a closure,
the representation created for it says you create a closure, but doesn't
say what code address goes into the closure, since the declarative debugger
does not (yet) need that info. I have taken other shortcuts as well.

> The new modules should be documented in compiler/notes/compiler_design.html.

Done.

> It would probably be a tiny bit nicer to make these procedures
> functions rather than predicates.

Some of the representation predicates have two output arguments;
they return an updated cell counter as well. I prefer uniformly using
predicates over using some predicates and some functions.

> It would probably be a good idea to abstract out the calls to
> term__var_to_int into a separate procedure prog_rep__represent_var.

Not usefully, since prog_rep__represent_var would need to return an int.

> > +% This module handles the generation of XXX
> 
> That comment is incomplete.

% This module handles the conversion of Mercury terms in the compiler
% into rvals we can give to llds_out.m in order to make those terms available
% at runtime in the program being compiled.

> > +:- pred get_functor_info(univ::in, functor_tag_info::out) is semidet.
> 
> You should document what that predicate does, and why it is nededed/what it
> is used by.

Done.

> >  	incr_hp_msg(Univ, 2, MR_PROC_LABEL, ""std_util:univ/0"");
> > +    MR_define_univ_fields(Univ, TypeInfo_for_T, Value);
> 
> Inconsistent indentation -- there should be a tab before the
> `MR_define_univ_fields'.

Fixed. The root cause is that some parts of std_util use 8-space indentation,
while others use 4-space, which makes it difficult to edit the file without
such problems.

> I think `MR_set_univ_fields' would be a clearer name.

In all its uses, the fields are defined once and never updated afterward.

> > +    Word            value;
> 
> s/Word/MR_Word/
> (likewise elsewhere)

The whole module was using those type names without MR_ prefixes. I have added
them.

> > +:- pragma c_code(get_du_functor_info(Univ::in, Where::out,
> > +    Ptag::out, Sectag::out, Args::out), will_not_call_mercury, "
> 
> It would be helpful to have a comment here explaining the meaning of
> the `Where' argument.
> 
> A general comment explaining what the predicate does and when
> it will fail could also be handy.

Done.

Thanks, Fergus.

Zoltan.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------