[m-rev.] for review: disable_warning [unknown_format_calls]

Julien Fischer jfischer at opturion.com
Wed Sep 7 20:26:59 AEST 2022


On Wed, 7 Sep 2022, Zoltan Somogyi wrote:

> On Wed, 7 Sep 2022 19:31:22 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
>>> But the other instances of disable_warning scopes for this warning
>>> *could* be avoided with a pragma such as you suggest.
>>
>> My intent was not avoid the disable_warnings scopes, but to check calls
>> to the wrapper predicates.
>
> I knew that, but the presence of such a pragma *should*, I think,
> disable the unknown_format_call warning, *for any call constructed
> using the nominated format_string/poly_types arguments*.

Ah, I see what you were getting at. That behaviour seems reasonable.

>> I have been bitten a non-trivial number of times by calls to log_msg/5
>> above containing invalid format strings. Had these been calls to, for
>> example, string.format the compiler would have caught such errors.
>
> I figured as much.
>
>>> I would propose that intead of format_args(2, 3) (I see no need for the []s),
>>> we have two separate and more descriptive arguments, like this:
>>>
>>> :- pragma check_format_call(pred(pd_debug_message/5),
>>>  format_string(2), poly_types(3)).
>>
>> I don't have any objections to that.  The reason I used a list was that,
>> in princple, you can certainly write predicates with multiple format
>> string and multiple list of poly_type arguments. (It's argubly, not a
>> particularly useful thing to do.)
>
> Ah. In that case, I propose that after the identification of the pred/func,
> the second argument should either have the format "format_string_poly_types(N, M)",
> where N & M are the argument numbers of the format string and the poly types
> respectively (for the usual use case), or a list of such terms (for the use case
> you just brought up). Having the format string and the poly types specified
> separately would work, or at least wouldn't work well, in the latter use case.
>
> Any objections?

No.

Julien.


More information about the reviews mailing list