[m-rev.] for review: mtc_union
Ian MacLarty
maclarty at cs.mu.OZ.AU
Thu Aug 11 23:29:08 AEST 2005
On Wed, 10 Aug 2005, Julien Fischer wrote:
>
> On Fri, 5 Aug 2005, Ian MacLarty wrote:
>
> > For review by anyone.
> >
> > Estimated hours taken: 10
> > Branches: main
> >
> > Implement mtc_union program. mtc_union takes a set of trace count files and
> > combines them into one trace count file. This is useful when slicing or dicing
> > lots of files as it means all the files don't have to be individually loaded
> > each time, which can take a long time.
> >
>
> I suggest adding a section to the user guide that describes all
> the tools we have for manipulating trace count files, slices, dices
> etc, as well as describing what they are and how they can be used.
>
Can I do that, along with adding mtc_slice and mtc_dice scripts and adding
--help options to all these tools, in another diff?
> > probably uses less memory.
> >
> > Store the number of tests each label is executed in with each
> > label. This must be done to preserve this informationn after
> s/informationn/information/
>
Fixed.
> > + % We could add a sanity check that LineNumber1 = LineNumber2, but that
> > + % would take time to no good purpose, since there isn't much we can do to
> > + % generate a meaningful message, and that situation doesn't necessarily
> > + % represent an error anyway. (Consider the case when the two trace files
> > + % are derived from sources that are identical except for the addition of a
> > + % comment.)
> > +
> That's not very clear - are you trying to explain why the "sanity check"
> is in fact not a sanity check?
>
I've changed the comment. See the interdiff at the end of this email.
> > - Result = RestResult
> > + ReadTCResult = io_error(IOError),
> > + Result = list_error_message("I/O error reading file " ++
> > + "`" ++ FileName ++ "': " ++ string.string(IOError))
> You might want to consider using io.error_message to get a better
> description of the error (and below as well).
>
Thanks. I've done that.
> > +write_trace_counts_to_file(FileType, TraceCounts, FileName, Res, !IO) :-
> > + io__open_output(FileName, Result, !IO),
> > + (
> > + Result = ok(FileStream),
> > + Res = ok,
> > + io.set_output_stream(FileStream, OldOutputStream, !IO),
> > + io.write_string(trace_count_file_id, !IO),
> > + write_trace_counts(FileType, TraceCounts, !IO),
> > + io__set_output_stream(OldOutputStream, _, !IO),
> > + io__close_output(FileStream, !IO)
>
> I suggest avoiding using different kinds of module qualifier in the
> body of the same predicate.
>
Fixed.
> > +%
> > +% Tool to combine several trace counts into one.
> > +%
> Author?
>
Fixed.
> > +%-----------------------------------------------------------------------------%
> > +
> > +:- module mtc_union.
> > +
> > +:- interface.
> > +
> > +:- import_module io.
> > +
> > +:- pred main(io::di, io::uo) is det.
> > +
>
> ...
>
> > +usage(!IO) :-
> > + io__write_strings([
> > + "Usage: mtc_union [-p] -o output_file file1 file2 ...\n",
> > + "The -p or --progress option causes each trace count ",
> > + "file name\n",
> I would change the name of that option to '--verbose' so that it is
> consistent with the other Mercury tools. For the same reason consider
> adding a '--help' option.
>
Done. I will add a --help option when I do the documentation and add
--help options to mslice and mdice if that's okay.
Here's the interdiff:
diff -u mdbcomp/trace_counts.m mdbcomp/trace_counts.m
--- mdbcomp/trace_counts.m 4 Aug 2005 08:53:59 -0000
+++ mdbcomp/trace_counts.m 11 Aug 2005 13:18:08 -0000
@@ -192,12 +192,10 @@
= line_no_and_count.
sum_counts_on_line(LC1, LC2) = LC :-
- % We could add a sanity check that LineNumber1 = LineNumber2, but that
- % would take time to no good purpose, since there isn't much we can do to
- % generate a meaningful message, and that situation doesn't necessarily
- % represent an error anyway. (Consider the case when the two trace files
- % are derived from sources that are identical except for the addition of a
- % comment.)
+ % We don't check that LineNumber1 = LineNumber2 since that does not
+ % necessarily represent an error. (Consider the case when the two trace
+ % files are derived from sources that are identical except for the addition
+ % of a comment.)
LC1 = line_no_and_count(LineNumber1, Count1, NumTests1),
LC2 = line_no_and_count(_LineNumber, Count2, NumTests2),
@@ -225,20 +223,22 @@
Result = list_ok(FileType, TraceCount)
;
ReadTCResult = io_error(IOError),
+ ErrMsg = io.error_message(IOError),
Result = list_error_message("IO error reading file " ++
- "`" ++ FileName ++ "': " ++ string.string(IOError))
+ "`" ++ FileName ++ "': " ++ ErrMsg)
;
ReadTCResult = open_error(IOError),
+ ErrMsg = io.error_message(IOError),
Result = list_error_message("IO error opening file " ++
- "`" ++ FileName ++ "': " ++ string.string(IOError))
+ "`" ++ FileName ++ "': " ++ ErrMsg)
;
- ReadTCResult = syntax_error(Error),
+ ReadTCResult = syntax_error(ErrMsg),
Result = list_error_message("Syntax error in file `" ++
- FileName ++ "': " ++ Error)
+ FileName ++ "': " ++ ErrMsg)
;
- ReadTCResult = error_message(Message),
+ ReadTCResult = error_message(ErrMsg),
Result = list_error_message("Error reading trace counts " ++
- "from file `" ++ FileName ++ "': " ++ Message)
+ "from file `" ++ FileName ++ "': " ++ ErrMsg)
)
;
Source = try_single_first,
@@ -306,20 +306,22 @@
TraceCounts, MainFileName, Stream, Result, !IO)
;
ReadTCResult = io_error(IOError),
+ ErrMsg = io.error_message(IOError),
Result = list_error_message("I/O error reading file " ++
- "`" ++ FileName ++ "': " ++ string.string(IOError))
+ "`" ++ FileName ++ "': " ++ ErrMsg)
;
ReadTCResult = open_error(IOError),
+ ErrMsg = io.error_message(IOError),
Result = list_error_message("I/O error opening file " ++
- "`" ++ FileName ++ "': " ++ string.string(IOError))
+ "`" ++ FileName ++ "': " ++ ErrMsg)
;
- ReadTCResult = syntax_error(Error),
+ ReadTCResult = syntax_error(ErrMsg),
Result = list_error_message("Syntax error in file `" ++
- FileName ++ "': " ++ Error)
+ FileName ++ "': " ++ ErrMsg)
;
- ReadTCResult = error_message(Message),
+ ReadTCResult = error_message(ErrMsg),
Result = list_error_message("Error reading trace counts " ++
- "from file `" ++ FileName ++ "': " ++ Message)
+ "from file `" ++ FileName ++ "': " ++ ErrMsg)
)
)
;
@@ -606,8 +608,8 @@
io.set_output_stream(FileStream, OldOutputStream, !IO),
io.write_string(trace_count_file_id, !IO),
write_trace_counts(FileType, TraceCounts, !IO),
- io__set_output_stream(OldOutputStream, _, !IO),
- io__close_output(FileStream, !IO)
+ io.set_output_stream(OldOutputStream, _, !IO),
+ io.close_output(FileStream, !IO)
;
Result = error(Error),
Res = error(Error)
diff -u slice/mtc_union.m slice/mtc_union.m
--- slice/mtc_union.m 4 Aug 2005 08:44:13 -0000
+++ slice/mtc_union.m 11 Aug 2005 13:20:56 -0000
@@ -5,6 +5,7 @@
%-----------------------------------------------------------------------------%
%
% Tool to combine several trace counts into one.
+% Main Author: Ian MacLarty.
%
%-----------------------------------------------------------------------------%
@@ -46,9 +47,8 @@
Args \= [],
OutputFile \= ""
->
- lookup_bool_option(OptionTable, show_progress,
- ShowProgress),
- union_trace_counts(ShowProgress, Args, 0, map.init,
+ lookup_bool_option(OptionTable, verbose, Verbose),
+ union_trace_counts(Verbose, Args, 0, map.init,
OutputFile, !IO)
;
usage(!IO)
@@ -98,7 +98,7 @@
usage(!IO) :-
io__write_strings([
"Usage: mtc_union [-p] -o output_file file1 file2 ...\n",
- "The -p or --progress option causes each trace count ",
+ "The -v or --verbose option causes each trace count ",
"file name\n",
"to be printed as it is added to the union.\n",
"file1, file2, etc can be trace count files or they\n",
@@ -108,7 +108,7 @@
%-----------------------------------------------------------------------------%
:- type option
- ---> show_progress
+ ---> verbose
; output_filename.
:- type option_table == option_table(option).
@@ -117,13 +117,13 @@
:- pred long_option(string::in, option::out) is semidet.
:- pred option_default(option::out, option_data::out) is multi.
-option_default(show_progress, bool(no)).
+option_default(verbose, bool(no)).
option_default(output_filename, string("")).
-short_option('p', show_progress).
+short_option('v', verbose).
short_option('o', output_filename).
-long_option("progress", show_progress).
+long_option("verbose", verbose).
long_option("out", output_filename).
%-----------------------------------------------------------------------------%
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------
More information about the reviews
mailing list