[m-rev.] for review: improvements to the coverage testing tool

Julien Fischer jfischer at opturion.com
Wed Apr 1 16:09:04 AEDT 2015


For review by anyone.

---------------------

Improvements to mcov.

slice/mcov.m:
 	Do not include labels from the standard library in coverage reports by
 	default.  Add a new command line option that can be used to force mcov
 	to include these labels.

 	XXX TODO: labels for opt_imported standard library procedures are still
 	showing up -- I will address this in a separate change.

 	Change the usage message to use the same format that mmc and mprof
 	use.

 	Set the exit status to a non-zero value if an error occurs.

 	Use io.format in more places.

 	Replace calls to error/1 with unexpected/3.

 	Add a new option, --flags-file, that allows options to be read
 	from a file (as with the identically named mmc option).

Julien.

diff --git a/slice/mcov.m b/slice/mcov.m
index 1c7a890..308bf2c 100644
--- a/slice/mcov.m
+++ b/slice/mcov.m
@@ -2,6 +2,7 @@
  % vim: ft=mercury ts=4 sw=4 expandtab
  %-----------------------------------------------------------------------------%
  % Copyright (C) 2006-2007, 2010-2012 The University of Melbourne.
+% Copyright (C) 2015 The Mercury team.
  % This file may only be copied under the terms of the GNU General
  % Public License - see the file COPYING in the Mercury distribution.
  %-----------------------------------------------------------------------------%
@@ -26,6 +27,7 @@
  :- implementation.

  :- import_module mdbcomp.
+:- import_module mdbcomp.builtin_modules.
  :- import_module mdbcomp.goal_path.
  :- import_module mdbcomp.prim_data.
  :- import_module mdbcomp.shared_utilities.
@@ -34,8 +36,9 @@

  :- import_module assoc_list.
  :- import_module bool.
-:- import_module getopt.
+:- import_module getopt_io.
  :- import_module int.
+:- import_module library.
  :- import_module list.
  :- import_module map.
  :- import_module maybe.
@@ -45,74 +48,92 @@
  :- import_module string.
  :- import_module term_io.

+%-----------------------------------------------------------------------------%
+
  main(!IO) :-
      unlimit_stack(!IO),
      io.command_line_arguments(Args0, !IO),
      OptionOps = option_ops_multi(short_option, long_option, option_default),
-    getopt.process_options(OptionOps, Args0, Args, GetoptResult),
+    getopt_io.process_options(OptionOps, Args0, Args, GetoptResult, !IO),
      (
          GetoptResult = ok(OptionTable),
+        ( if lookup_bool_option(OptionTable, help, yes)
+        then long_usage(!IO)
+        else do_coverage_testing(OptionTable, Args, !IO)
+        )
+    ;
+        GetoptResult = error(GetoptErrorMsg),
+        write_error_message(GetoptErrorMsg, !IO)
+    ).
+
+%-----------------------------------------------------------------------------%
+
+:- pred do_coverage_testing(option_table(option)::in, list(string)::in,
+    io::di, io::uo) is det.
+
+do_coverage_testing(OptionTable, Args, !IO) :-
+    (
+        Args = [_ | _],
+        lookup_bool_option(OptionTable, verbose, Verbose),
+        read_and_union_trace_counts(Verbose, Args, _NumTests, FileTypes,
+            TraceCounts, MaybeReadError, !IO),
+        io.stderr_stream(StdErr, !IO),
          (
-            Args = [_ | _],
-            lookup_bool_option(OptionTable, verbose, Verbose),
-            read_and_union_trace_counts(Verbose, Args, _NumTests, FileTypes,
-                TraceCounts, MaybeReadError, !IO),
-            stderr_stream(StdErr, !IO),
-            (
-                MaybeReadError = yes(ReadErrorMsg),
-                io.write_string(StdErr, ReadErrorMsg, !IO),
-                io.nl(StdErr, !IO)
-            ;
-                MaybeReadError = no,
-                set.to_sorted_list(FileTypes, FileTypeList),
-                ( FileTypeList = [single_file(BaseType)] ->
-                    BaseType = base_count_file_type(Kind, _Program),
-                    (
-                        Kind = user_nonzero,
-                        io.write_string(StdErr, kind_warning, !IO)
-                    ;
-                        Kind = user_all
-                    )
+            MaybeReadError = yes(ReadErrorMsg),
+            write_error_message(ReadErrorMsg, !IO)
+        ;
+            MaybeReadError = no,
+            set.to_sorted_list(FileTypes, FileTypeList),
+            ( FileTypeList = [single_file(BaseType)] ->
+                BaseType = base_count_file_type(Kind, _Program),
+                (
+                    Kind = user_nonzero,
+                    io.write_string(StdErr, kind_warning, !IO)
                  ;
-                    io.write_string(StdErr, consistency_warning, !IO)
-                ),
-                lookup_bool_option(OptionTable, detailed, Detailed),
-                lookup_accumulating_option(OptionTable, modules, Modules),
+                    Kind = user_all
+                )
+            ;
+                io.write_string(StdErr, consistency_warning, !IO)
+            ),
+            lookup_bool_option(OptionTable, detailed, Detailed),
+            lookup_accumulating_option(OptionTable, modules, Modules),
+            (
+                Modules = [],
+                lookup_bool_option(OptionTable, ignore_stdlib, IgnoreStdLib),
                  (
-                    Modules = [],
-                    RestrictToModules = no
+                    IgnoreStdLib = no,
+                    RestrictToModules = module_restriction_none
                  ;
-                    Modules = [_ | _],
-                    ModuleSyms = list.map(string_to_sym_name, Modules),
-                    RestrictToModules = yes(set.list_to_set(ModuleSyms))
-                ),
-                lookup_string_option(OptionTable, output_filename, OutputFile),
-                ( OutputFile = "" ->
+                    IgnoreStdLib = yes,
+                    RestrictToModules = module_restriction_no_stdlib
+                )
+            ;
+                Modules = [_ | _],
+                ModuleSyms = list.map(string_to_sym_name, Modules),
+                RestrictToModules = module_restriction_user(set.list_to_set(ModuleSyms))
+            ),
+            lookup_string_option(OptionTable, output_filename, OutputFile),
+            ( OutputFile = "" ->
+                write_coverage_test(Detailed, RestrictToModules,
+                    TraceCounts, !IO)
+            ;
+                io.tell(OutputFile, OpenRes, !IO),
+                (
+                    OpenRes = ok,
                      write_coverage_test(Detailed, RestrictToModules,
                          TraceCounts, !IO)
                  ;
-                    io.tell(OutputFile, OpenRes, !IO),
-                    (
-                        OpenRes = ok,
-                        write_coverage_test(Detailed, RestrictToModules,
-                            TraceCounts, !IO)
-                    ;
-                        OpenRes = error(OpenErrorMsg),
-                        io.write_string(StdErr, "Error opening " ++
-                            "file `" ++ OutputFile ++ "'" ++ ": " ++
-                            string(OpenErrorMsg), !IO),
-                        io.nl(StdErr, !IO)
-                    )
+                    OpenRes = error(OpenError),
+                    io.error_message(OpenError, OpenErrorMsg),
+                    io.format(StdErr, "Error opening file `%s': %s\n",
+                        [s(OutputFile), s(OpenErrorMsg)], !IO),
+                    io.set_exit_status(1, !IO)
                  )
              )
-        ;
-            Args = [],
-            usage(!IO)
          )
      ;
-        GetoptResult = error(GetoptErrorMsg),
-        io.write_string(GetoptErrorMsg, !IO),
-        io.nl(!IO)
+        Args = [],
+        short_usage(!IO)
      ).

  :- func kind_warning = string.
@@ -145,28 +166,41 @@ consistency_warning =
  :- type trace_counts_list ==
      assoc_list(proc_label_in_context, proc_trace_counts).

-:- pred write_coverage_test(bool::in, maybe(set(module_name))::in,
+:- type module_restriction
+    --->    module_restriction_none
+            % No module restriction.
+
+    ;       module_restriction_no_stdlib
+            % All modules except those in the standard library.
+
+    ;       module_restriction_user(set(module_name)).
+            % Only the user-specified set of modules given by the argument.
+
+:- pred write_coverage_test(bool::in, module_restriction::in,
      trace_counts::in, io::di, io::uo) is det.

  write_coverage_test(Detailed, RestrictToModules, TraceCountMap, !IO) :-
      map.to_assoc_list(TraceCountMap, TraceCounts0),
      (
-        RestrictToModules = no,
+        RestrictToModules = module_restriction_none,
          TraceCounts = TraceCounts0
      ;
-        RestrictToModules = yes(Modules),
+        RestrictToModules = module_restriction_no_stdlib,
+        list.negated_filter(in_stdlib_module, TraceCounts0, TraceCounts)
+    ;
+        RestrictToModules = module_restriction_user(Modules),
          list.filter(in_module_set(Modules), TraceCounts0, TraceCounts)
      ),
      (
          Detailed = no,
          collect_zero_count_local_procs(TraceCounts, ZeroCountProcs),
-        sort(ZeroCountProcs, SortedZeroCountProcs),
+        list.sort(ZeroCountProcs, SortedZeroCountProcs),
          io.write_string("Unexecuted procedures:\n\n", !IO),
          list.foldl(write_proc_info, SortedZeroCountProcs, !IO)
      ;
          Detailed = yes,
          collect_zero_count_local_labels(TraceCounts, [], ZeroCountLabels),
-        sort(ZeroCountLabels, SortedZeroCountLabels),
+        list.sort(ZeroCountLabels, SortedZeroCountLabels),
          io.write_string("Unexecuted labels:\n\n", !IO),
          list.foldl(write_label_info, SortedZeroCountLabels, !IO)
      ).
@@ -178,6 +212,13 @@ in_module_set(Modules, ProcLabelInContext - _) :-
      ProcLabelInContext = proc_label_in_context(Module, _, _),
      set.member(Module, Modules).

+:- pred in_stdlib_module(pair(proc_label_in_context, proc_trace_counts)::in)
+    is semidet.
+
+in_stdlib_module(ProcLabelInContext - _) :-
+    ProcLabelInContext = proc_label_in_context(Module, _, _),
+    is_std_lib_module_name(Module, _).
+
  %-----------------------------------------------------------------------------%

  :- pred collect_zero_count_local_procs(trace_counts_list::in,
@@ -291,10 +332,7 @@ is_local_proc(ProcLabel) :-

  write_proc_info(ProcInfo, !IO) :-
      ProcInfo = proc_info(FileName, LineNumber, ProcLabel),
-    io.write_string(FileName, !IO),
-    io.write_char(':', !IO),
-    io.write_int(LineNumber, !IO),
-    io.write_string(": ", !IO),
+    write_context(FileName, LineNumber, !IO),
      write_proc_label_for_user(ProcLabel, !IO),
      io.nl(!IO).

@@ -302,14 +340,16 @@ write_proc_info(ProcInfo, !IO) :-

  write_label_info(LabelInfo, !IO) :-
      LabelInfo = label_info(FileName, LineNumber, ProcLabel, PathPort),
-    io.write_string(FileName, !IO),
-    io.write_char(':', !IO),
-    io.write_int(LineNumber, !IO),
-    io.write_string(": ", !IO),
+    write_context(FileName, LineNumber, !IO),
      write_proc_label_for_user(ProcLabel, !IO),
      write_path_port_for_user(PathPort, !IO),
      io.nl(!IO).

+:- pred write_context(string::in, int::in, io::di, io::uo) is det.
+
+write_context(FileName, LineNumber, !IO) :-
+    io.format("%s:%d: ", [s(FileName), i(LineNumber)], !IO).
+
  :- pred write_proc_label_for_user(proc_label::in, io::di, io::uo) is det.

  write_proc_label_for_user(ProcLabel, !IO) :-
@@ -318,20 +358,18 @@ write_proc_label_for_user(ProcLabel, !IO) :-
              _DeclModuleSym, Name, Arity, Mode),
          (
              PredOrFunc = pf_predicate,
-            io.write_string("pred ", !IO)
+            PredOrFuncStr = "pred"
          ;
              PredOrFunc = pf_function,
-            io.write_string("func ", !IO)
+            PredOrFuncStr = "func"
          ),
-        term_io.quote_atom(Name, !IO),
-        io.write_string("/", !IO),
-        io.write_int(Arity, !IO),
-        io.write_string("-", !IO),
-        io.write_int(Mode, !IO)
+        QuotedName = term_io.quoted_atom(Name),
+        io.format("%s %s/%d-%d",
+            [s(PredOrFuncStr), s(QuotedName), i(Arity), i(Mode)], !IO)
      ;
          % We don't record trace counts in special preds.
          ProcLabel = special_proc_label(_, _, _, _, _, _),
-        error("write_proc_label_for_user: special_pred")
+        unexpected($file, $pred, "special_pred")
      ).

  :- pred write_path_port_for_user(path_port::in, io::di, io::uo) is det.
@@ -347,50 +385,116 @@ write_path_port_for_user(port_and_path(Port, Path), !IO) :-

  %-----------------------------------------------------------------------------%

-:- pred usage(io::di, io::uo) is det.
+:- pred short_usage(io::di, io::uo) is det.

-usage(!IO) :-
+short_usage(!IO) :-
+    io.progname_base("mcov", ProgName, !IO),
+    library.version(Version, FullArch),
      io.write_strings([
-        "Usage: mcov [-d] [-v] [-m module] [-o output_file] file1 file2 ...\n",
-        "The -d or --detailed option causes the printing of a report for\n",
-        "each label that has not been executed, even if some other code\n",
-        "has been executed in the same procedure.\n",
-        "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 should be trace count files.\n",
-        "If one or more -m or --module options are given, then the output\n",
-        "will be restricted to the modules named by their arguments.\n",
-        "The argument of the -o or --output-file option gives the name\n",
-        "of the output file.\n"],
-        !IO).
+        "Mercury Coverage Testing Tool, version ", Version, ", on ", FullArch, ".\n",
+        "Copyright (C) 2006-2007, 2010-2012 The University of Melbourne\n",
+        "Copyright (C) 2015 The Mercury team\n",
+        "Usage: ", ProgName, " [<options>] [<files>]\n",
+        "Use `", ProgName, " --help' for more information.\n"
+    ], !IO).
+
+:- pred long_usage(io::di, io::uo) is det.
+
+long_usage(!IO) :-
+    library.version(Version, FullArch),
+    io.format(
+        "Name: mcov -- Mercury Coverage Testing Tool, version %s, on %s\n",
+        [s(Version), s(FullArch)], !IO),
+    io.write_string("Copyright: Copyright (C) 2006-2007, 2010-2012 " ++
+        "The University of Melbourne\n", !IO),
+    io.write_string("           Copyright (C) 2015 " ++
+        "The Mercury team\n", !IO),
+    io.write_string("Usage: mcov [<options>] <arguments>\n", !IO),
+    io.write_string("Arguments:\n", !IO),
+    io.write_string("\tArguments are assumed to Mercury trace count files.\n", !IO),
+    io.write_string("Options:\n", !IO),
+    write_tabbed_lines([
+        "-?, -h, --help",
+        "\tPrint help about using mcov (on the standard output) and exit without",
+        "\tdoing any further processing",
+        "-v, --verbose",
+        "\tPrint the name of each trace count file as it is added to the union",
+        "-d, --detailed",
+        "\tPrint a report for each label that has not been executed, even",
+        "\tif some other code has been executed in the same procedure.",
+        "-m <module>, --module <module>",
+        "\tRestrict the output to the module named by the argument.",
+        "\tMultiple module options accumulate.",
+        "-o <file>, --output-file <file>",
+        "\tPrint output to the file specified by the argument.",
+        "\tBy default the output will be printed to the standard output.",
+        "--flags <file>, --flags-file <file>",
+        "\tTake options from the specified file, and handle them as if they",
+        "\twere specified on the command line.",
+        "--no-ignore-stdlib",
+        "\tInclude information about labels in the Mercury standard library",
+        "\tin the reports."
+    ], !IO).
+
+:- pred write_tabbed_lines(list(string)::in, io::di, io::uo) is det.
+
+write_tabbed_lines([], !IO).
+write_tabbed_lines([Str | Strs], !IO) :-
+    io.format("\t%s\n", [s(Str)], !IO),
+    write_tabbed_lines(Strs, !IO).

  %-----------------------------------------------------------------------------%

  :- type option
-    --->    detailed
+    --->    help
+    ;       verbose
+    ;       detailed
      ;       modules
      ;       output_filename
-    ;       verbose.
+    ;       flags_file
+    ;       ignore_stdlib.

  :- type option_table == option_table(option).

  :- pred short_option(character::in, option::out) is semidet.
+
+short_option('?', help).
+short_option('h', help).
+short_option('v', verbose).
+short_option('d', detailed).
+short_option('m', modules).
+short_option('o', output_filename).
+
  :- pred long_option(string::in, option::out) is semidet.
+
+long_option("help",             help).
+long_option("verbose",          verbose).
+long_option("detailed",         detailed).
+long_option("module",           modules).
+long_option("output-file",      output_filename).
+long_option("flags",            flags_file).
+long_option("flags-file",       flags_file).
+long_option("ignore-stdlib",    ignore_stdlib).
+
  :- pred option_default(option::out, option_data::out) is multi.

+option_default(help,            bool(no)).
+option_default(verbose,         bool(no)).
  option_default(detailed,        bool(no)).
  option_default(modules,         accumulating([])).
  option_default(output_filename, string("")).
-option_default(verbose,         bool(no)).
+option_default(flags_file,      file_special).
+option_default(ignore_stdlib,   bool(yes)).
+
+%-----------------------------------------------------------------------------%

-short_option('d',               detailed).
-short_option('m',               modules).
-short_option('o',               output_filename).
-short_option('v',               verbose).
+:- pred write_error_message(string::in, io::di, io::uo) is det.

-long_option("detailed",         detailed).
-long_option("module",           modules).
-long_option("output-file",      output_filename).
-long_option("verbose",          verbose).
+write_error_message(Msg, !IO) :-
+    io.stderr_stream(Stderr, !IO),
+    io.format(Stderr, "%s\n", [s(Msg)], !IO),
+    io.set_exit_status(1, !IO).

  %-----------------------------------------------------------------------------%
+:- end_module mcov.
+%-----------------------------------------------------------------------------%



More information about the reviews mailing list