[m-rev.] for post-commit review: a start on getting type repn info from .int files

[Peter Wang]

On Mon, 26 Jul 2021 12:10:02 +1000 Julien Fischer <jfischer at opturion.com> wrote:
> 
> Hi Zoltan,
> 
> On Sun, 25 Jul 2021, Zoltan Somogyi wrote:
> 
> >
> > 2021-07-22 16:53 GMT+10:00 "Zoltan Somogyi" <zoltan.somogyi at runbox.com>:
> >> 2021-07-22 14:43 GMT+10:00 "Peter Wang" <novalazy at gmail.com>:
> >>>> Then how did the .int files get generated ok for Julien and me
> >>>> during a workspace bootcheck?
> >>>> I don't know. That is a mystery best investigated on your respective
> >>> machines.
> >>
> >> Ok, I will look into that.
> >
> > The attached diff should fix the symptoms for now; it is for post-commit review
> > by anyone. Fixing the underlying problems will take more work. The log message
> > details these problems.
> 
> The diff looks fine and seems to have resolved the issues on
> testing.mercurylang.org.
> 
> > After you have read that log message, I have some questions/proposals.
> >
> > First, should we change  tools/bootcheck so that --test-params is the default?
> > Julien and I both apparantly forgot its existence; we believed that the
> > presence of --intermod-opt in Mmake.stage.params would cause the tests
> > to be done with --intermod-opt, when that is true ONLY with --test-params.
> > I propose that we should.
> 
> Agreed.

Agreed. I forgot about --test-params as well, because I've long used a
wrapper script that always passes --test-params.

> 
> > Second, what should we do with test cases in tests/invalid which generate
> > errors when one tries to make dependencies for them? I see three avenues
> > of approach. (1) Ensure that we never make dependencies for them. (2)
> > Make dependencies for them, but redirect them to /dev/null, and ignore
> > the nonzero exit status. (3) Add an option to the compiler that will suppress
> > both the error messages and the nonzero exit status. And I also see two ways
> > we could ensure that they get the proper treatment. (A) Create a new variable
> > (say NODEPEND) in the Mmakefile listing their names, and a  "($NODEPEND):%.err:%m"
> > rule, or (B) moving them to a new test directory. On this question, I am agnostic,
> > though (3) would require nontrivial changes in the handling of errors during
> > dependency generation, which is one of the few parts of the compiler
> > which prints error messages as it goes along, instead of collecting them up
> > and printing them all at once.
> 
> I think they should be moved to separate tests directory regardless of
> which of the above is chosen; supporting two different ways of handling
> the tests in the same directory just complicates the test
> infrastructure. Doing that should allow for (1), which seems to be
> the simpler of the three options.

I agree. "Anything" to simplify our makefiles.

> 
> With regard to the way the compiler prints out error messages during
> dependency generation: I assume that its current behaviour is just
> for historical reasons?  (i.e. collecting and printing them all at
> once would be the more desirable behaviour anyway)
> 
> > Third, for the other test cases in tests/invalid*, i.e. the ones for which
> > making dependencies works without any errors, should (1) we make the
> > dependencies, and then use the <module>.ints mmake targets, and
> > maybe related targets, to make the required dependencies, or
> > (2) just execute mmc --make-short-interface/--make-interface commands
> > directly in the relevant mmake rules? I would prefer (1).
> 
> I also prefer 1.
> 

Also 1.

> > Fourth, tests/invalid/Mmakefile contains this:
> >
> > # XXX: with `mmake --use-mmc-make' the ".DEFAULT:" rule seems to take
> > # precedence over "%.err: %.m" rules.
> > # XXX: the reason we run the $(MCM) command twice is to avoid doubled up
> > # error messages, once while making interface files, then the module proper.
> > # The second time the command is run, only one set of error messages
> > # should appear.
> > $(addsuffix .err,$(PROGS)):
> >        -$(MCM) $@
> >        if $(MCM) -r $@ > /dev/null 2>&1 ; \
> >        then false; \
> >        else true; \
> >        fi
> >
> > Does anyone know why the code throws away the output of the
> > second invocation of $(MCM), when the comment implies that
> > we actually want to throw away the output of the first invocation?
> > That code was first introduced in commit 2005, in commit
> > 82722524e484468ebffe4b23ac48370f2e39d046, though it has
> > been reformatted since, including replacing the original % comment
> > characters with a make-appropriate #.
> 
> No idea for my part.  Peter, does your memory stretch back
> that far?

The exit status of the first invocation is ignored:

    -$(MCM) $@

The second invocation forces a rebuild:

    if $(MCM) -r $@ > /dev/null 2>&1

which overwrites the .err files from the first invocation.

Peter