[m-dev.] For review: declarative debugging back end (1/3)

[Mark Anthony BROWN]

Hi,

> > Added files:
> > 
> > compiler/evaluation_tree_gen:
> > 	Implements the compiler stage that adds the code to generate
> > 	evaluation trees (stage 23). 
> 
> Please put the `.m's in the log message.
> 
> You should document the new modules in compiler/notes/compiler_design.html.
> 
> Documenting the numerical stage numbers isn't that helpful; these are
> subject to change over time.  What would be more useful to document
> is which stage it comes after and which stage it comes before, and why.
> 

Ok.

> > compiler/mercury_compile:
> > 
> > 	If --generate-evaluation-tree is set, then calculate goal paths
> > 	and then call evaluation_tree_gen, in stage 23 <fnord>.
> 
> <fnord>?
> 

I was being facetious :) :)

> > compiler/options:
> > 
> > 	Added the option --generate-evaluation-tree, of type bool.
> 
> You need to also modify doc/user_guide.texi to document that option.
> 

Sure.

> > library/library:
> > 
> > 	Added evaluation_tree to the list of imports.
> 
> Should you also modify doc/Mmakefile to exclude that module
> from the library reference manual?

Yes, atleast at this stage.

In the future, IMO, the interface to the front end should be a visible
part of the (a?) library.  As noted by some researchers, the back end
can be reused for various different purposes, so ultimately it would
be good to allow users access to the generated EDTs.

> 
> You should also add somethinng to either the NEWS or the WORK_IN_PROGRESS file.
> 

Ok.  I'll mention it in WORK_IN_PROGRESS, because you can't do anything
useful with it yet.

> > --- make_hlds.m	1998/07/08 20:56:38	1.269
> > +++ make_hlds.m	1998/07/12 07:03:53
> > @@ -46,6 +46,14 @@
> > +        %
> > +        % Add a new exported type to an existing HLDS structure.  This
> > +        % is used by evaluation_tree_gen.
> > +        %
> > +:- pred add_new_exported_type(module_info, tvarset, type_defn, condition,
> > +                term__context, module_info, io__state, io__state).
> > +:- mode add_new_exported_type(in, in, in, in, in, out, di, uo) is det.
> > +
> > @@ -62,6 +70,8 @@
> >  :- import_module code_util, unify_proc, special_pred, type_util, mode_util.
> >  :- import_module mercury_to_mercury, passes_aux, clause_to_proc, inst_match.
> >  :- import_module fact_table, purity, goal_util, term_util, export, llds.
> > +:- import_module typecheck, post_typecheck, modes, mode_info.
> > +:- import_module switch_detection, det_analysis.
> 
> If that predicate runs all those later compilation stages, then
> the documentation for it should say so.

Ok, it does run them.

> 
> I think make_hlds.m is not the best place to add this predicate.
> make_hlds.m really shouldn't need to import all those modules for
> later stages.  Hmm...
> 
> > +add_new_exported_type(Module0, TVarSet, TypeDefn, Cond, Context, Module) -->
> > +        module_add_type_defn(Module0, TVarSet, TypeDefn, Cond, Context,
> > +                        item_status(exported, must_be_qualified), Module1),
> > +
> > +        globals__io_get_globals(Globals),
> > +        { convert_type_defn(TypeDefn, Globals, Name, Args, _Body) },
> > +        { list__length(Args, Arity) },
> > +        { TypeId = Name - Arity },
> 
> Why not just make it `add_type' and give it an `item_status' parameter?
> That would make it more generally useful.
> 

Ok.  the 'new_' part of the name was meant to indicate that we were
adding a type from scratch (ie doing all the semantic analysis), as
opposed to adding a type in the usual way (module_add_type_defn).
Should I call it `add_new_type', `add_type_from_scratch', something else,
or would you still prefer `add_type'?

> For d.u. types, the call to convert_type_defn here will unnecessarily
> call assign_constructor_tags.  I think you should instead define a predicate
> 
> 	get_name_args(TypeDefn, Name, Args)
> 
> and use that instead of convert_type_defn.
> 
> > +        { special_pred_list(SpecialPreds) },
> > +        update_special_preds(Module1, SpecialPreds, TypeId, Module).
> 
> To avoid the probelm with make_hlds.m importing later compilation stages,
> you could add `add_type' to hlds.m but delete these two lines, and
                              ^^^^^^
Did you mean this?  Or did you mean make_hlds.m or hlds_something.m?

> instead add the code for update_special_preds in some other module --
> perhaps just in evaluation_tree_gen.

Ok.

> 
> options.m:
> > +        io__write_string("\t--generate-evaluation-tree\n"),
> > +        io__write_string("\t\tInclude code to generate evaluation trees in the\n"),
> > +        io__write_string("\t\tC code output by the compiler.  These can be analysed\n"),
> > +        io__write_string("\t\tby a declarative debugger.\n"),
> >          io__write_string("\t--generate-bytecode\n"),
> >          io__write_string("\t\tOutput a bytecode form of the module for use\n"),
> >          io__write_string("\t\tby an experimental debugger.\n"),
> 
> I suggest you add "(not yet implemented) " before "declarative debugger".
> 
> Also for consistency I suggest you change "an experimental debugger"
> in the documentation for --generate-bytecode to "a (not yet implemented)
> bytecode debugger".
> 

Sure.

Cheers,
Mark
-- 
Mark Brown  (dougl at cs.mu.oz.au)             |  Any technology that is
MEngSc student,                             |  distinguishable from magic
Dept of Computer Science, Melbourne Uni     |  is insufficiently advanced