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

[Zoltan Somogyi]

On Wed, 7 Sep 2022 16:45:35 +1000 (AEST), Julien Fischer <jfischer at opturion.com> wrote:
> Unrelated: is it feasible to extend the checking done by compiler/format_call.m
> to check wrappers around io.format etc. like pd_debug_message?

It is doable, but only for some use cases, not all.

Specifically, your proposal would not help two of the scopes in DIFF.ufc2:

1. In parse_tree_out_info.m, there is output_format/5. Putting this pragma on that
predicate would not help, because it is never called directly. Instead of calling
output_format, mercury_to_mercury.m calls add_format/5, the typeclass method
that is implemented by output_format. So the pragma would have to be placed
on add_format, and (at the moment) we do not allow pragmas to be placed
on typeclass methods. (The attached diff deletes this method.)

2. In string.m, we have

+    disable_warning [unknown_format_calls] (
+        format("%#." ++ int_to_string(Prec) ++ "g", [f(Float)], Tmp)
+    ),

In this case, the reason why we need the scope is that the result of
int_to_string(Prec) is not a constant, because Prec is an input. For this,
the right fix is not the pragma, but an expansion of formats to
support the specification of parameters such as precisions
not in the format string, but in polytypes. C's printf supports
something similar, e.g. with printf("%.*s", 3, "abcdef").
This may be worthwhile, but I think it is far from urgent.

But the other instances of disable_warning scopes for this warning
*could* be avoided with a pragma such as you suggest.

The pragma would need its own field in pred_sub_info,
which is distributed fat, but only a small amount.

> For example,
> if we add a pragma like:
> 
>      :- pragma check_format_call(pred(pd_debug_message/5), [format_args(2, 3)]).
> 
> where format_args(2, 3) identifies the arguments containing the format string
> and its corresponding list of poly_types. (GCC supports a function attribute
> that does something similar for printf()).

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)).

If we get agreement on the syntax, I will write up the diff for the refman entry,
and implement the change once there is agreement on *that*.
 
> This diff looks fine.

Thank you.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.af
Type: application/octet-stream
Size: 179 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20220907/3f0871ea/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.af
Type: application/octet-stream
Size: 4695 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20220907/3f0871ea/attachment-0003.obj>