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

[Julien Fischer]

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?

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

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

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

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

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

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

Looks good otherwise.

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