[m-rev.] for post-commit review: fix infinite loop bug in compiler

[Zoltan Somogyi]

For review by anyone.

I also have some questions raised by this diff.

Q1: if two or more insts, say i1. i2 and i3 are mutually recursive, then
by definition, the definitions of all of them are circular. After this diff,
we generate one error message for each such inst. Should we instead
generate a single error message for the cycle as a whole?

Q1a: if the answer to Q1 is yes, then is it enough for the one error message
to contain the context of just one inst (either the context with the smallest
line number, or the context for the lexicographically first inst name), or
should that one error message have one component for each inst giving
its context?

My preferred answer to  Q1 is yes. I am agnostic on Q1a, primarily because
(a) the contexts of some insts may refer to .intN files, which I don't think we
want to print, but (b) non-.intN contexts would be useful, and (c) printing
only the non-.intN contexts would look strange.

Q2: At the moment, all the inst names printed in error messages are fully
module qualified. This is definitely overkill for the inst being complained about,
if it is locally defined. I see three alternatives:

A2a: report all insts in fully qualified form, as now.
A2b: report all insts in unqualified form.
A2c: report all insts from the current module in unqualified form,
and all other insts in fully qualified form.
A2d: report all insts from the current module *that are not also defined
in builtin.m* in unqualified form, and all other insts in fully qualified form.

The point of A2d is that insts and modes defined in builtin.m are
the only insts/modes whose form in error messages is the same as their
unqualified form, since we routinely strip that module from module
qualifications.

My preferred answer to Q2 is A2d.

Q3: Instead of the current terse error message, which has the form
"circular equivalence {inst 1,insts i1 and i2}", should we have a longer
error message that explains *why* circularity is a problem?

My preferred answer is yes. I would prefer wording such as
"Inst name i1 expands to an inst containing itself, which means that
processing any reference to i1 would require an infinite sequence
of expansions", adding "through insts i2 and i3" after "expands to"
in the case of mutual recursion. Note that we can't say "would expand
to an infinitely large inst", because e.g. ":- inst inf == inf" wouldn't.

The same would apply, mutatis mutandis, to modes.

What are your preferences?

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.circ
Type: application/octet-stream
Size: 1100 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20211011/2975db27/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.circ
Type: application/octet-stream
Size: 19823 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20211011/2975db27/attachment-0003.obj>