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

Julien Fischer jfischer at opturion.com
Mon Jul 26 12:10:02 AEST 2021


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.

> 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.

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.

> 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?

Julien.


More information about the reviews mailing list