[m-rev.] for review: --output-stdlib-grades

Julien Fischer jfischer at opturion.com
Fri Jan 21 20:43:50 AEDT 2022


Hi Zoltan,

On Fri, 21 Jan 2022, Zoltan Somogyi wrote:


> Add the new option --output-stdlib-grades.
> 
> compiler/mercury_compile_main.m:
>     Implement the new option. Doing so without duplicating the existing
>     code tree of detect_libgrades required factoring out its reusable parts.
>
>     Do not pass I/O states to computations that do not need them.
>
>     Add XXXs for some questionable past decisions.
> 
> compiler/op_mode.m:
>     Define the new op_mode selected by the new option.
> 
> compiler/options.m:
> doc/user_guide.texi:
>     Add and document the new option, and revert my screw-up of the
>     documentation of --output-libgrades.
> 
> compiler/options_file.m:
>     Make the interface of another predicate less error prone.
> 
> 
> diff --git a/compiler/mercury_compile_main.m b/compiler/mercury_compile_main.m
> index 9d9cddfa1..e9f657cef 100644
> --- a/compiler/mercury_compile_main.m
> +++ b/compiler/mercury_compile_main.m

...

> @@ -462,13 +476,17 @@ maybe_dump_options_file(ArgsGlobals, Variables, !IO) :-
>
>  %---------------------%
> 
> -% Enable the compile-time trace flag "debug-detect-libgrades" to enable
> -% debugging messages for library grade detection in the very verbose output.
> -
> -:- pred detect_libgrades(globals::in, maybe(list(string))::in,
> -    list(string)::out, io::di, io::uo) is det.
> -
> -detect_libgrades(Globals, MaybeConfigMerStdLibDir, GradeOpts, !IO) :-
> +:- pred maybe_detect_stdlib_grades(globals::in, options_variables::in,
> +    maybe1(set(string))::out, list(string)::out, io::di, io::uo) is det.
> +
> +maybe_detect_stdlib_grades(Globals, Variables,
> +        MaybeStdlibGrades, StdlibGradeOpts, !IO) :-
> +    % Enable the compile-time trace flag "debug-detect-libgrades" to enable
> +    % debugging messages for library grade detection in the very verbose
> +    % output.
> +    % XXX If the compiler is compiled with this trace flag, then enabling -v
> +    % but not -V will tell you only that this predicate has been invoked,
> +    % but not its result. I (zs) would find that annoying.

Feel free to change it so the result is printed with only -v enabled; I don't
think there is a particularly good reason to only print it  with -V enabled.
(Probably, last I used it was with --very-verbose and I never noticed otherwise.)


> @@ -477,77 +495,153 @@ detect_libgrades(Globals, MaybeConfigMerStdLibDir, GradeOpts, !IO) :-
>              maybe_write_string(Verbose, "% Detecting library grades ...\n",
>                  !TIO)
>          ),
> -        globals.lookup_bool_option(Globals, very_verbose, VeryVerbose),
> -        % NOTE: a standard library directory specified on the command line
> -        % overrides one set using the MERCURY_STDLIB_DIR variable.
> -        ( if
> -            % Was the standard library directory set on the command line?
> -            globals.lookup_maybe_string_option(Globals,
> -                mercury_standard_library_directory, MaybeStdLibDir),
> -            MaybeStdLibDir = yes(MerStdLibDir)
> -        then
> -            do_detect_libgrades(VeryVerbose, MerStdLibDir, GradeOpts, !IO)
> -        else if
> -            % Was the standard library directory set using the
> -            % MERCURY_STDLIB_DIR variable?
> -            MaybeConfigMerStdLibDir = yes([MerStdLibDir])
> -        then
> -            do_detect_libgrades(VeryVerbose, MerStdLibDir, GradeOpts, !IO)
> -        else
> -            GradeOpts = []
> +        find_mercury_stdlib(Globals, Variables, MaybeMerStdLibDir, !IO),
> +        % If we cannot find the Mercury standard library directory,
> +        % we return the error message(s) explain the reason for that

s/explain/explaining/

> +        % in MaybeStdlibGrades, which one our caller pays attention to,
> +        % but NOT in StdlibGradeOpts, which is for another of our callers.
> +        (
> +            MaybeMerStdLibDir = ok1(MerStdLibDir),
> +            do_detect_libgrades(MerStdLibDir, StdlibGrades, !IO),
> +            MaybeStdlibGrades = ok1(StdlibGrades)
> +        ;
> +            MaybeMerStdLibDir = error1(Specs),
> +            MaybeStdlibGrades = error1(Specs),
> +            set.init(StdlibGrades)

...

> +    % Where is the Mercury standard library?
> +    %
> +    % NOTE: A standard library directory specified on the command line
> +    % overrides one set using the MERCURY_STDLIB_DIR variable.
> +    %
> +:- pred find_mercury_stdlib(globals::in, options_variables::in,
> +    maybe1(string)::out, io::di, io::uo) is det.
> +
> +find_mercury_stdlib(Globals, Variables, MaybeMerStdLibDir, !IO) :-
> +    ( if
> +        % Was the standard library directory set on the command line?
> +        globals.lookup_maybe_string_option(Globals,
> +            mercury_standard_library_directory, MaybeStdLibDir),
> +        MaybeStdLibDir = yes(MerStdLibDir)
> +    then
> +        can_you_read_dir(MerStdLibDir, MaybeMerStdLibDir, !IO)
> +    else
> +        % Was the standard library directory set using the
> +        % MERCURY_STDLIB_DIR variable?
> +        lookup_mercury_stdlib_dir(Variables, MaybeConfigMerStdLibDir),
> +        (
> +            MaybeConfigMerStdLibDir = ok1(MerStdLibDirs),
> +            (
> +                MerStdLibDirs = [MerStdLibDir],
> +                can_you_read_dir(MerStdLibDir, MaybeMerStdLibDir, !IO)
> +            ;
> +                MerStdLibDirs = [],
> +                Pieces = [words("Error: the location of the directory"),
> +                    words("that holds the Mercury standard library"),
> +                    words("is not specified either by the value of any"),
> +                    quote("mercury-stdlib-dir"), words("option"),

s/mercury-stdlib-dir/--mercury-stdlib-dir/

> +                    words("to the compiler, nor by any definition of the"),
> +                    quote("MERCURY_STDLIB_DIR"), words("variable in the"),
> +                    quote("Mercury.options"), words("file."), nl],

MERCURY_STDLIB_DIR should be set in the global Mercury.config file; it is not
usually set in a Mercury.options file.

> +                Spec = simplest_no_context_spec($pred, severity_error,
> +                    phase_options, Pieces),
> +                MaybeMerStdLibDir = error1([Spec])
> +            ;
> +                MerStdLibDirs = [_, _ | _],
> +                Pieces = [words("Error: the definition of the"),
> +                    quote("MERCURY_STDLIB_DIR"), words("variable in the"),
> +                    quote("Mercury.options"), words("file"),

Likewise.

> +                    words("contains more than one string."), nl],
> +                Spec = simplest_no_context_spec($pred, severity_error,
> +                    phase_options, Pieces),
> +                MaybeMerStdLibDir = error1([Spec])
> +            )
> +        ;
> +            MaybeConfigMerStdLibDir = error1(Specs),
> +            MaybeMerStdLibDir = error1(Specs)
> +        )
>      ).

...

> diff --git a/doc/user_guide.texi b/doc/user_guide.texi
> index fc4525dff..c5e328bf5 100644
> --- a/doc/user_guide.texi
> +++ b/doc/user_guide.texi
> @@ -8330,7 +8330,7 @@ the Mercury runtime system and Mercury standard library
>  will be installed in only a subset of the possible grades;
>  you can find out which grades these are
>  by invoking the Mercury compiler
> -with the @samp{--output-libgrades} option.
> +with the @samp{--output-stdlib-grades} option.
>  Attempting to use a grade which has not been installed
>  will result in an error at link time.
>  (The error message will typically be something like

You also need to document the --output-stdlib-grades option here.

The rest looks fine.

Julien.


More information about the reviews mailing list