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

[Julien Fischer]

On Mon, 7 Feb 2005, Ian MacLarty wrote:

> On Mon, Feb 07, 2005 at 12:58:58AM +1100, Julien Fischer wrote:
> >
> > On Sun, 6 Feb 2005, Ian MacLarty wrote:
> >
> > > For review by anyone.
> > >
> > > Estimated hours taken: 40
> > > Branches: main
> > >
> > > Add mdb `dice' command which reads in a set of passing trace counts and a
> > > failing trace count and prints a comparison table.  The table can be sorted
> > > by various metrics and is useful for finding parts of a program executed in
> > > a failing run, but not in passing runs.
> > >
> > The log message should mention that this eventually intendted to be used
> > as part of the declartive debugger.
> >
>
> Although the declarative debugger will use some of the code in
> dice.m the `dice' mdb command is intended to be an independent feature.
> So I don't think mentioning the declarative debugger is necessary or
> relevant.
>
Fair enough.

> >
> > > +	% be buggy based on how many times it was executed in passing and
> > > +	% failing test runs.
> > > +	%
> > > +suspicion_ratio(PassCount, FailCount) =
> > > +	% PassCount + FailCount should never be zero since if a label
> > > +	% isn't executed in any tests then it wouldn't be included in the dice.
> > > +	float(FailCount) / float(PassCount + FailCount).
> > > +
> > I suggest putting a check that PassCount + FailCount \= 0 there.
> >
>
> Do you mean it should fail if the denominator is zero?  This should
> never be the case since then the label wouldn't be included in
> the dice.  Or do you mean I should throw an exception if the denominator
> is zero?  Won't an exception get thrown anyway if I try to divide by
> zero?
>
Yes - ignore that.

> > > +			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.


> > > +    for (module_num = 0; module_num < num_modules; module_num++) {
> > > +        module = MR_module_infos[module_num];
> > > +        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;
> > > +                    id = &proc->MR_sle_user;
> > > +                    if (MR_streq(id->MR_user_name, Name) &&
> > > +                            id->MR_user_arity == Arity &&
> > > +                            id->MR_user_mode == ModeNo) {
> > > +                        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) {
> > > +                            SUCCESS_INDICATOR = MR_find_context(label,
> > > +                                &filename, &LineNo);
> > > +                            MR_TRACE_USE_HP(
> > > +                                MR_make_aligned_string(FileName,
> > > +                                    (MR_String) filename);
> > > +                            );
> > > +                            goto end;
> > > +                        }
> > > +                    }
> > > +                }
> > > +            }
> > > +        }
> > > +    }
> > > +
> > 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.

> > > + at sp 1
> > > + at item dice [-p at var{filename}] [-f at var{filename}] [-n at var{num}] [-s[pPfFsS]+] [-o @var{filename}]
> > > + at kindex dice (mdb command)
> > > +Display a program dice on the screen.
> > > + at sp 1
> > > +A dice is a comparison between some successful test runs of the program and a
> > > +failing test run.  Before using the @samp{dice} command one or more passing
> > > +execution summaries and one failing execution summary should be generated.
> > > +This can be done by compiling the program with deep tracing enabled (either by
> > > +compiling in the .debug or .decldebug grades or with the @samp{--trace deep}
> >
> > 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.

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
--------------------------------------------------------------------------