[m-rev.] for post-commit review: stop putting never-read entries in a map

[Julien Fischer]

On Sun, 14 Dec 2025 at 09:24, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
>
> On Sat, 13 Dec 2025 14:50:10 +1100, Julien Fischer <jfischer at opturion.com> wrote:
>
> > On Fri, 12 Dec 2025 at 09:04, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> > > @@ -171,29 +170,26 @@
> > >      % (such as Mercury.config and Mercury.options).
> > >  :- type env_optfile_variables
> > >      --->    env_optfile_variables(
> > > -                % This field is for the variable values we get from
> > > -                % the small set of environment variables that
> > > -                % we pay attention to.
> > > +                % This field contains the set of all the environment variables
> > > +                % of the current process. Most of these are not related to
> > > +                % Mercury, which means we cannot pre-process them all.
> >
> >    *at* all
>
> No, I meant just "all", though it is clear from your question that
> the sentence did not get across what I wanted it to get across.
> I am replacing it with:
>
>
> Most of these are not related to Mercury, so preprocessing all environment
> variable values would yield lots of diagnostics about settings that have
> nothing to do with Mercury.

That's clearer.

> > > +            % Checking now means that we detect and report errors in the value
> > > +            % of an environment variable as early as possible after that
> > > +            % envvar is set to a value we do not consider acceptable.
> > > +            %
> > > +            % On the other hand, if two envvars relevant to a Mercury program,
> > > +            % say MCFLAGS-mod_a and MCFLAGS-mod_b, both have the same error,
> > > +            % but only MCFLAGS-mod_a appears in a Mercury.options file, then
> > > +            % the code here would report the error only for the first envvar,
> > > +            % leaving the second to be reported by lookup_variable_words.
> > > +            % That would seem to violate the law of least astonishment.
> >
> > > +            % Reporting any errors here and now preserves old behavior.
> > > +            % XXX The question is: *should* we preserve this old behavior?
> >
> > On balance, I would be inclined to preserve the existing behaviour.
>
> In the meantime, I thought of a possible third approach that gets
> the best of both worlds, *most* of the time. The idea is to process
> not all envvars, but all envvars whose names fit one of the patterns
> that Mercury pays attention to.
>
> Some patterns would be fixed strings: MCFLAGS and EXTRA_MCFLAGS
> would be two examples.
>
> Other patterns would be prefix matches: MCFLAGS- and CFLAGS-
> would be examples of these.
>
> We would need lists of both kinds of patterns, but scripts/Mmake.{vars,rules}
> should give us that.

It will give you some of that.  There are some options variables that
are not supported
by mmake, e.g. GCC_FLAGS, MSVC_FLAGS and CLANG_FLAGS spring to mind.
(Perhaps we should rename these to MC_GCC_FLAGS etc, so they more obviously
"belong" to the Mercury compiler.)

> The "most of the time" qualifier is there because it is theoretically possible
> for someone to defined an envvar named e.g. MCFLAGS-xyz *without* intending it
> to apply to a Mercury module named xyz. However, I don't think this is likely to be
> a significant problem in practice.

The specific instance where someone is defining an environment
variable with a dash
in its name is already a very rare occurrence because the shell on
Unix-like systems
makes it difficult to do that.

> Opinions?

That approach seems fine.

Julien.