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

[Fergus Henderson]

This change looks good.  I have quite a few comments, but they're
mostly minor issues.

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.  As with standard Prolog, this
would only be available for procedures which the programmer declared
that they wanted it.  (In standard prolog, the declaration used is `:-
dynamic', but we'd want to use something different, since `:- dynamic'
also allows assert/retract, which we don't want to support.)

On 25-Sep-2000, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> compiler/prog_rep.m:
> 	A new module, containing the code that converts goals from HLDS
> 	to a term in the format we want to put in the layout structure.
> 
> compiler/static_term.m:
> 	A new module, containing the code that converts Mercury terms
> 	to the LLDS rval we need to give to llds_out.m.

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

> library/std_util.m:
> 	Add a mechanism for static_term.m to use in converting terms into
> 	rvals. This mechanism uses RTTI information to desconstruct terms,

s/desconstruct/deconstruct/

> trace/mercury_trace_internal.m:

s/.m/.c/

> +:- pred prog_rep__represent_detism(determinism::in,
> +	detism_rep::out) is det.
...
> +:- pred prog_rep__represent_cons_id(cons_id::in,
> +	cons_id_rep::out) is det.
> +
> +prog_rep__represent_cons_id(cons(SymName, _), Rep) :-
> +	prog_rep__represent_sym_name(SymName, Rep).
> +prog_rep__represent_cons_id(int_const(Int), Rep) :-
> +	string__int_to_string(Int, Rep).
> +prog_rep__represent_cons_id(float_const(Float), Rep) :-
> +	string__float_to_string(Float, Rep).
> +prog_rep__represent_cons_id(string_const(String), Rep) :-
> +	string__append_list(["""", String, """"], Rep).
> +prog_rep__represent_cons_id(pred_const(_, _, _), Rep) :-
> +	Rep = "$pred_const".
> +prog_rep__represent_cons_id(code_addr_const(_, _), Rep) :-
> +	Rep = "$code_addr_const".
> +prog_rep__represent_cons_id(type_ctor_info_const(_, _, _), Rep) :-
> +	Rep = "$type_ctor_info_const".
> +prog_rep__represent_cons_id(base_typeclass_info_const(_, _, _, _), Rep) :-
> +	Rep = "$base_typeclass_info_const".
> +prog_rep__represent_cons_id(tabling_pointer_const(_, _), Rep) :-
> +	Rep = "$tabling_pointer_const".
> +
> +:- pred prog_rep__represent_sym_name(sym_name::in, string::out) is det.

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

> +prog_rep__represent_goal_expr(unify(_, _, _, Uni, _), GoalInfo, InstMap0,
> +		ModuleInfo, Rep) :-
> +	(
> +		Uni = assign(Target, Source),
> +		term__var_to_int(Target, TargetRep),
> +		term__var_to_int(Source, SourceRep),
> +		AtomicGoalRep = unify_assign_rep(TargetRep, SourceRep)

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.

> +++ static_term.m	Sun Sep 24 02:32:48 2000
...
> +% File: static_term.m.
> +% Author: zs.
> +%
> +% This module handles the generation of XXX

That comment is incomplete.

> +++ library/std_util.m	2000/09/24 02:45:49
> @@ -508,6 +508,24 @@
>  	%
>  :- pred deconstruct(T::in, string::out, int::out, list(univ)::out) is det.
>  
> +:- implementation.
> +:- interface.
> +
> +% The rest of the interface is for use by implementors only.
> +
> +:- type functor_tag_info
> +        --->    functor_integer(int)
> +        ;       functor_float(float)
> +        ;       functor_string(string)
> +        ;       functor_enum(int)
> +        ;       functor_local(int, int)
> +        ;       functor_remote(int, int, list(univ))
> +        ;       functor_unshared(int, list(univ))
> +        ;       functor_notag(univ)
> +        ;       functor_equiv(univ).
> +
> +:- 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.

> @@ -1004,26 +1024,20 @@
>  	% Allocate heap space, set the first field to contain the address
>  	% of the type_info for this type, and then store the input argument
>  	% in the second field.
> -:- pragma c_code(type_to_univ(Type::di, Univ::uo), will_not_call_mercury, "
> +:- pragma c_code(type_to_univ(Value::di, Univ::uo), will_not_call_mercury, "
>  	incr_hp_msg(Univ, 2, MR_PROC_LABEL, ""std_util:univ/0"");
> -	MR_field(MR_mktag(0), Univ, UNIV_OFFSET_FOR_TYPEINFO)
> -		= (Word) TypeInfo_for_T;
> -	MR_field(MR_mktag(0), Univ, UNIV_OFFSET_FOR_DATA)
> -		= (Word) Type;
> +    MR_define_univ_fields(Univ, TypeInfo_for_T, Value);

Inconsistent indentation -- there should be a tab before the
`MR_define_univ_fields'.

I think `MR_set_univ_fields' would be a clearer name.

> @@ -1998,9 +2009,7 @@
>  	** Create a univ.
>  	*/
>  	incr_hp_msg(Term, 2, MR_PROC_LABEL, ""std_util:univ/0"");
> -	MR_field(MR_mktag(0), Term, UNIV_OFFSET_FOR_TYPEINFO) =
> -		(Word) type_info;
> -	MR_field(MR_mktag(0), Term, UNIV_OFFSET_FOR_DATA) = new_data;
> +    MR_define_univ_fields(Term, type_info, new_data);

Inconsistent indentation -- there should be a tab before the
`MR_define_univ_fields'.

> +:- pred get_enum_functor_info(Univ::in, Int::out) is semidet.
> +
> +:- pragma c_code(get_enum_functor_info(Univ::in, Enum::out),
> +    will_not_call_mercury, "
> +{
> +    MR_TypeInfo     type_info;
> +    MR_TypeCtorInfo type_ctor_info;
> +    Word            value;

s/Word/MR_Word/
(likewise elsewhere)

> +:- pred get_du_functor_info(univ::in, int::out, int::out, int::out,
> +    list(univ)::out) is semidet.
> +
> +:- 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.

Apart from that, this 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.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------