[m-rev.] for review: fix duplicate installed grade checks

[Julien Fischer]

Hi Zoltan,

On Fri, 10 Mar 2023, Zoltan Somogyi wrote:

> Fix calls to maybe_check_libraries_are_installed.
> 
> compiler/mercury_compile_main.m:
>     Compiler invocations that generate an executable, and thus do linkingg,

Delete the extra 'g' there.

>     should check that the selected grade's libraries are in fact available.
>     In non-"mmc --make" invocations, this is the job of mercury_compile_main.m.
>
>     Once upon a time, we used to do this once per compiler argument.
>     A diff later intended to change this so that the check was done
>     only once, and not repeated for every compiler argument, but it
>     did it wrong. It did add a single central check (the do-it-once part), but
>
>     - it did it only in the case where --filenames-from-stdin was not
>       specified, and
>
>     - it did not delete the do-it-for-every argument code.
>
>     For invocations with --filenames-from-stdin, the checks were still
>     needed, though only once. For invocations without --filenames-from-stdin,
>     the extra check was only repeated overhead. (The duplicate error messages
>     would have been deleted when they were sorted.)
>
>     Fix this by doing the check only once, whether --filenames-from-stdin
>     is specified or not.
>
>     Document why doing this is a reasonable thing to do. Add an XXX about
>     a possible improvement.
>
>     Resolve a conjecture in an old comment.



> diff --git a/compiler/mercury_compile_main.m b/compiler/mercury_compile_main.m
> index 0cf889133..5d363df2d 100644
> --- a/compiler/mercury_compile_main.m
> +++ b/compiler/mercury_compile_main.m
> @@ -727,54 +727,82 @@ do_op_mode_args(ProgressStream, ErrorStream, Globals, OpModeArgs,
>          FileNamesFromStdin, DetectedGradeFlags, OptionVariables,
>          OptionArgs, Args, !HaveReadModuleMaps, !Specs, !IO) :-
>      globals.lookup_bool_option(Globals, invoked_by_mmc_make, InvokedByMake),
> +    (
> +        InvokedByMake = no,
> +        % We used to do this check once per compiler arg, which was
> +        % quite wasteful. However, there was an almost-justifiable
> +        % reason for that. The check used to be done in the now-deleted
> +        % predicate process_compiler_arg_build, the part of what is now
> +        % setup_and_process_compiler_arg that happens after the setup.
> +        % This meant that maybe_check_libraries_are_installed was called
> +        % not with the original globals (which is Globals here), but with
> +        % the globals created by setup_for_build_with_module_options.
> +        % However, the only options in the globals structure that
> +        % maybe_check_libraries_are_installed pays attention to are
> +        %
> +        % - global settings, such as library_install_check and

I think you mean libgrade_install_check there.

> +        %   mercury_libraries, and
> +        %
> +        % - grade options.
> +        %
> +        % No sensible Mercury.options file will touch the value of

I suggest:

     No sensible Mercury.options file will contain module-specific options
     that touch the value of ...

> +        % any option in either of those categories. It shouldn't be able
> +        % to touch the values of the relevant options in the first category,
> +        % because they don't have any names which would allow them to be
> +        % specified.

...

> +        % We do this check only when InvokedByMake = no, because during
> +        % compiler invocations that *set* --invoked-by-mmc-make,
> +        % make_linked_target in make.program_target.m should have
> +        % done it already.

A possible improvement here might to encode the value of --invoked-by-mmc-make
in the op_mode, rather than have it be a separate flag that affects the op_mode.

That's fine otherwise.

Julien.