[m-rev.] for review: add --module option for dice mdb command

[Ian MacLarty]

On Sun, Feb 13, 2005 at 12:44:56AM +1100, Julien Fischer wrote:
> 
> On Sat, 12 Feb 2005, Ian MacLarty wrote:
> 
> > For review by anyone.
> >
> > Estimated hours taken: 6
> > Branches: main
> >
> > Add a --module option to the mdb dice command to limit output to a specific
> > module or package.
> >
> Is it possible to add a --exclude-modules option that would exclude
> output from specific modules?
> 

Yes it's possible.  But can it wait for another diff?

> > Write out the trace counts even if the program aborts with an exception.
> >
> s/aborts with an exception/aborts because of an uncaught exception/
> (and below)
> 

Okay.

> > Index: browser/dice.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/browser/dice.m,v
> > retrieving revision 1.1
> > diff -u -r1.1 dice.m
> > --- browser/dice.m	10 Feb 2005 04:10:27 -0000	1.1
> > +++ browser/dice.m	10 Feb 2005 06:08:37 -0000
> > @@ -51,7 +51,7 @@
> >  :- pred read_trace_counts_list(string::in, read_trace_counts_list_result::out,
> >  	io::di, io::uo) is det.
> >
> > -	% read_dice_to_string(PassFiles, FailFile, SortStr, N, DiceStr,
> > +	% read_dice_to_string(PassFiles, FailFile, SortStr, N, Module, DiceStr,
> >  	% 	Problem, !IO).
> >  	% Read the trace_counts in the list of files in the file named
> >  	% PassFiles, interpreting them as passing slices; read the
> > @@ -61,13 +61,15 @@
> >  	% SortStr can be any combination of the letters "sSpPfP" and
> >  	% indicates how the dice is to be sorted.  See the documentation
> >  	% for the `dice' command in the user guide for an explaination of the
> > -	% sort string.  If there was a problem reading the trace_counts then
> > -	% Problem will contain a string describing the problem encountered and
> > -	% DiceStr will be the empty string, otherwise Problem will be the
> > -	% empty string.
> > +	% sort string.  If Module is not the empty string then only labels
> > +	% from the named module will be included in the dice string, otherwise
> > +	% all modules will be included.  If there was a problem reading the
> > +	% trace_counts then Problem will contain a string describing the
> > +	% problem encountered and DiceStr will be the empty string, otherwise
> > +	% Problem will be the empty string.
> >  	%
> >  :- pred read_dice_to_string(string::in, string::in, string::in, int::in,
> > -	string::out, string::out, io::di, io::uo) is det.
> > +	string::in, string::out, string::out, io::di, io::uo) is det.
> >
> I think that it would be better if Problem had type maybe(string)
> and returned yes(ProblemString) if there was a problem and no if there
> was no problem.  The approach using an empty string to indicate no
> error seems less robust.

Yes but I did it this way to make it easier to use in the C code.

> 
> > Index: doc/user_guide.texi
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
> > retrieving revision 1.417
> > diff -u -r1.417 user_guide.texi
> > --- doc/user_guide.texi	10 Feb 2005 04:10:28 -0000	1.417
> > +++ doc/user_guide.texi	12 Feb 2005 04:12:30 -0000
> > @@ -3391,7 +3391,7 @@
> >  Clears the histogram printed by @samp{histogram_exp},
> >  i.e.@: sets the counts for all depths to zero.
> >  @sp 1
> > - at item dice [-p at var{filename}] [-f at var{filename}] [-n at var{num}] [-s[pPfFsS]+] [-o @var{filename}]
> > + at item dice [-p at var{filename}] [-f at var{filename}] [-n at var{num}] [-s[pPfFsS]+] [-o @var{filename}] [-m @var{module}]
> >  @kindex dice (mdb command)
> >  Display a program dice on the screen.
> >  @sp 1
> > @@ -3499,6 +3499,9 @@
> >  will be written to the specified file instead of being displayed on the
> >  screen.  Note that the file will be overwritten without warning if it
> >  already exists.
> > + at sp 1
> > +The @samp{-m} or @samp{--module} option limits the output to the given module
> > +or package.
> Mercury doesn't formally have packages so I would change that to say
> "limits the output to the given module and its submodules, if any".
> 

Okay.

> > Index: mdbcomp/prim_data.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/mdbcomp/prim_data.m,v
> > retrieving revision 1.3
> > diff -u -r1.3 prim_data.m
> > --- mdbcomp/prim_data.m	10 Feb 2005 04:10:29 -0000	1.3
> > +++ mdbcomp/prim_data.m	11 Feb 2005 04:22:15 -0000
> > @@ -132,6 +132,12 @@
> >  :- pred sym_name_to_string(sym_name::in, string::out) is det.
> >  :- func sym_name_to_string(sym_name) = string.
> >
> > +	% is_submodule(SymName1, SymName2).
> > +	% True iff SymName1 is a submodule of SymName2.
> > +	% For example mod1.mod2.mod3 is a submodule of mod1.mod2.
> > +	%
> > +:- pred is_submodule(sym_name::in, sym_name::in) is semidet.
> > +
> Since this is for use with module names rather sym_names in general
> it might be better to declare that as:
> 
> :- pred is_submodule(module_name::in, module_name::in) is semidet.
> 

Good idea.

> > @@ -187,7 +193,12 @@
> >  	sym_name_to_string(ModuleSym, Separator, ModuleName),
> >  	string__append_list([ModuleName, Separator, Name], QualName).
> >
> > +is_submodule(SymName, SymName).
> > +is_submodule(qualified(SymName1, _), SymName2) :-
> > +	is_submodule(SymName1, SymName2).
> > +
> I would prefer `SymNameA' and `SymNameB' there.  Having a sequence
> of numbered variables makes it looks as though there is some sort
> of relationship between them.
> 

Okay.

> > Index: runtime/mercury_trace_base.h
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/runtime/mercury_trace_base.h,v
> > retrieving revision 1.43
> > diff -u -r1.43 mercury_trace_base.h
> > --- runtime/mercury_trace_base.h	10 Feb 2005 04:10:30 -0000	1.43
> > +++ runtime/mercury_trace_base.h	11 Feb 2005 03:51:04 -0000
> > @@ -110,6 +110,15 @@
> >  extern	void	MR_trace_write_label_exec_counts(FILE *fp);
> >
> >  /*
> > +** MR_trace_write_label_exec_counts_to_file does the same job as
> > +** MR_trace_write_label_exec_counts, except that it also opens the file.
> > +** It's signature allows it to be registered with
> s/It's/Its/
> 

DOH!

Cheers,

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