[m-rev.] for review: add mdb `dice' command

[Julien Fischer]

On Mon, 7 Feb 2005, Ian MacLarty wrote:

> On Mon, Feb 07, 2005 at 03:25:23PM +1100, Julien Fischer wrote:
> > > > > +			SpecialPredId = compare,
> > > > > +			Name = "__Compare__"
> > > > > +		;
> > > > > +			SpecialPredId = initialise,
> > > > > +			Name = "__Initialise__"
> > > > > +		),
> > > > The module compiler/special_pred contains predicates for handling the
> > > > names of special preds.  I suggest moving the relevant bits into the
> > > > mdbcomp library and using them here.
> > > >
> > >
> > > Okay.  I've moved special_pred_description/2 from
> > > compiler/special_pred.m to mdbcomp/prim_data.m, made it a function, and
> > > used it in dice.m
> > >
> >
> > special_pred_description/2 is going to give you a different form
> > of the name than that you originally had.  I was thinking of
> > special_pred_name_arity/3.
> >
>
> special_pred_name_arity/3 will also give a different name, but the
> issue's not very important since compiler generated predicates aren't
> included in the trace counts anyway.  I've used special_pred_name_arity
> instead since the names it gives are shorter.
>

Ok.

> > > > The amount of nesting and lack of whitespace here make this block of
> > > > code very difficult to understand.  Would it be possible to factor some
> > > > of this out into separate functions?
> > > >
> > >
> > > What do you mean by lack of whitespace?  Where would you like to see
> > > more whitespace?
> > >
> > Perhaps lack of whitespace wasn't a good way of putting it - my main point
> > was that a great big lump of C code like that is fairly unreadable.
> >
>
> What about this:
>
>     for (module_num = 0; module_num < num_modules; module_num++) {
>         module = MR_module_infos[module_num];
>         /*
>         ** Check if the label occurs in this module.
>         */
>         if (MR_streq(Module, module->MR_ml_name)) {
>             num_files = module->MR_ml_filename_count;
--->


>             for (file_num = 0; file_num < num_files; file_num++) {
>                 file = module->MR_ml_module_file_layout[file_num];
>                 num_labels = file->MR_mfl_label_count;
--->

>                 for (label_num = 0; label_num < num_labels; label_num++) {
>                     label = file->MR_mfl_label_layout[label_num];
>                     proc = label->MR_sll_entry;
I think you should probably add comments at the beginning of those for
loops (where I've indicated as well).

>                     id = &proc->MR_sle_user;
>                     /*
>                     ** Check if the proc name, arity and mode are the same as
> 		      ** the name, arity and mode for the label we are interested
> 		      ** in.
>                     */
>                     if (MR_streq(id->MR_user_name, Name) &&
>                             id->MR_user_arity == Arity &&
>                             id->MR_user_mode == ModeNo) {
>                         /*
>                         ** Check if the path/port is the same as the
>                         ** label we are interested in.
>                         */
>                         MR_TRACE_CALL_MERCURY(
>                             MR_MDB_same_path_port(
>                                 (MR_String) MR_label_goal_path(label),
>                                 label->MR_sll_port, PathPort, &are_same);
>                         );
>                         if (are_same) {
>                             /*
>                             ** We have found the label, so return the
>                             ** filename and line number.
>                             */
>                             filename = file->MR_mfl_filename;
>                             LineNo = file->MR_mfl_label_lineno[label_num];
>                             SUCCESS_INDICATOR = MR_TRUE;
>                             MR_TRACE_USE_HP(
>                                 MR_make_aligned_string(FileName,
>                                     (MR_String) filename);
>                             );
>                             goto end;
>                         }
>                     }
>                 }
>             }
>         }
>     }
>
> Note I also removed the call to MR_find_context, since it wasn't really
> necessary.
>
Ok.

> > > > The .decldebug grade is not currently documented.
> > > >
> > >
> > > Okay, I commented out the .decldebug bit.
> > >
> > Might it not be better to just document the .decldebug grades since they
> > are now useful for the subterm dependency tracking anyway?  Perhaps,
> > as a separate change though.
> >
>
> Yes that's a seperate change.  I noticed that the .decldebug grade
> documentation is commented out in several places, so I need to go
> through and uncomment it.
>
The main places are in options.m and in the user guide.

Cheers,
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------