[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