[m-rev.] for post-commit review: fix github issue #118

[Zoltan Somogyi]

2023-03-18 16:43 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
> 1. The diff below contains comments suggesting it was added (presumably
>    written before you realised it wouldn't work?)

That presumption is correct.

> 2. The new test case should go somewhere, even if we currently lack the
>    means to run it; not adding it simply risks losing it.  I suggest we
>    add the directory tests/manual_invalid and use it to to house such
>    orphan test cases. (At the least, this lets us check that everything in
>    such a directory works before a release.)

The attached diff adds it to a new test directory called invalid_manual.
(There are several test dirs named invalid_X for some X.)

>> The algorithm we use to decide what goes into .int0 files is very primitive,
>> being inherited almost unchanged from Fergus's "everything is a list of items"
>> design, and consists of copying items from the parse tree of the .m file
>> almost unchanged to the .int0 file. We could change this, as we have already
>> started doing for invalid combinations of type, inst and mode declarations
>> in .int/.int2 files, but that requires more discussion up front.

Actually, in this case the error is a .int file, not .int0, but the same point
stands.

>> tests/invalid/magicbox.err_exp:
>> tests/invalid/try_detism.err_exp:
>>     Expect an error message about the invalid determinism of a procedure
>>     with I/O state args. Previously, we did not generate an error message
>>     for these issues.
> 
> These test cases changes were not included in the diff, but what you'e
> committed looks fine.

It seems that what I sent to the list was not the final diff.

>> +        % Almost all error messages this predicate will generate
>> +        % will refer to a procedure that is local to the module being compiled,
>> +        % and for these, pring the module name would only be clutter.
> 
> s/pring/printing/

Fixed.

> That looks fine otherwise.

Thanks. The attached diff is for you to review.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.gh118b
Type: application/octet-stream
Size: 516 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20230318/631edeff/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.gh118b
Type: application/octet-stream
Size: 5566 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20230318/631edeff/attachment-0003.obj>