[m-rev.] for review: pragma format_call

Zoltan Somogyi zoltan.somogyi at runbox.com
Sat Sep 24 08:48:44 AEST 2022


2022-09-23 12:36 GMT+10:00 "Julien Fischer" <jfischer at opturion.com>:
>> This diff has a ZZZ in add_pragma.m, on code that checks the status
>> of the new pragma the same way that we check the status of other
>> declarative pragmas: we generate an error if the predicate (or function)
>> to which the pragma applies is exported, but the pragma itself is not exported.
>>
>> I think this test is wrong. We should generate an error in the *opposite*
>> case: the pragma is exported, but the pred/func it applies to is not exported.
> 
> We should definitely be doing that.
> 
>> The case when the pred/func is exported but the declarative pragma is not
>> exported should merit only a warning, because the other modules that
>> import that pred/func can survive without knowing that e.g. that pred/func
>> is obsolete, or that calls to it can be checked the same way as calls to e.g.
>> string.format.
> 
> That seems reasonable.

I will do both these things in a separate diff.

> Add a comment saying that this last call is intended to generate a warning
> about there begin unknown format values in the call to io.format.

Done, both this and your other suggestions.

> The testing here is not comprehensive enough. We should also have:
> 
> 1. A test of the case where the new pragam has a single format_string_values
> argument (i.e. the common case), since the error message has a different form
> in that case.
> 
> 2. A multi-module test case that checks that calls to imported predicates that
> are the subject of format_call pragmas have the checks applied.
> 
> 3. A format_call pragma applied to a multi-moded predicate.  (I will add
> such a test after this is committed, since I can extract something from G12
> for it.)

I have done the first two, and leave the third to you.

> The diff looks fine otherwise.

Thank you.

Zoltan.


More information about the reviews mailing list