[m-rev.] for post-commit review: expanding @file cmd line args
Zoltan Somogyi
zoltan.somogyi at runbox.com
Fri Apr 3 17:04:50 AEDT 2026
On Fri, 3 Apr 2026 15:15:39 +1100, Julien Fischer <jfischer at opturion.com> wrote:
> > + ArgResult = apr_failure(OptionTable, OoMArgSpecs),
> > + ArgSpecs = one_or_more_to_list(OoMArgSpecs),
> > + io.write_string(ProgressStream, "mmc:\n", !IO),
>
> Hardcoding the name to "mmc" is not ideal; for the MSVC port it will be
> "mercury", not "mmc". Why not just use the result of io.progname_base/4
> as we do elsewhere? E.g.
>
> io.progname_base("mercury_compile", ProgName, !IO),
> io.format("ProgressStream, "%s:\n", [s(ProgName)], !IO)
>
> That will return the most appropriate exectuable name: "mmc" on most systems,
> "mercury" on Windows and "mercury_compile" if the executable is run directly.
There are two issues here.
The first is: as this diff shows, there are ten test cases that test what we print here.
These will each need one expected output file for each possible string we can print here.
For two possible strings, and even three, this is manageable (though not ideal).
If more are possible, it may not be. (We cannot use an option that says "always print this
here", because one of reasons we can get apr_failure here is that option processing
has failed.)
The second is: I think printing the name of the script, either mmc or mercury,
that invoked the executable is way more useful for users than printing the name of
the executable. From the users' point of view, the latter is an irrelevant detail.
That is why I don't think io.progname_base can help us here.
We could print "mercury compiler:". That is not the name of the script the user invoked,
but it will nevertheless identify where the output is coming from sufficiently uniquely,
regardless of the platform.
> > + write_error_specs_opt_table(ProgressStream, OptionTable,
> > + ArgSpecs, !IO),
> > + % In most cases, ArgSpecs should contain errors, so the above
> > + % should have set the exit status to 1. However, in cases such as
> > + % tests/invalid_options_file/undefined_var.m, it contains
> > + % only warnings.
> > + %
> > + % XXX In such cases, should we get our caller to return apr_success?
> > + % And if so, should we do so only with --no-halt-at-warn, or
> > + % with --halt-at-warn as well?
> > + io.set_exit_status(1, !IO)
>
> Unless I have missed something here, compilation should proceed with
> --no-halt-at-warn (i.e. we should return apr_success) and not proceed with
> --halt-at-warn. That would be the least suprising behaviour.
If we choose to pay attention to --halt-at-warn, that is exactly what I am proposing.
However, we can choose not to pay attention to it. ("do so" means "return apr_success"
above.)
The issue here is that --halt-at-warn is about the compiler setting a failure-indicating
exit status that prevents either mmake or mmc --make from continuing. It is silent
about what happens *within* the compiler invocation. From the very beginning, the
compiler halted early if it found some kinds of problems (such as references to
undefined types), while it continued after other kinds of problems (such as references
to undefined predicates and functions). The question that XXX is asking is about
which category any warnings in ArgSpecs, which mostly warn about undefined
make variables in Mercury.options files, should belong to.
As I see them, the choices about what we can do in the presence of such warnings,
but NO ERRORS, are
- always print the warnings and stop (the current choice),
- print the warnings (either now or later), and stop only with --halt-at-warn, and
- print the warnings, and continue to the rest of the compilation regardless
of the value halt_at_warn.
As far as I can see, the current approach was never explicitly chosen; it just happened
to follow the existing path that was intended for errors, not warnings.
> The rest looks fine.
Thanks.
Zoltan.
More information about the reviews
mailing list