On 6/27/21 10:24 PM, Tuan Le Quang via Gcc-patches wrote:
Hi,

Currently, format attribute can be used to do type-checking for arguments
with respect to  a format string. However, only functions with a C-style
ellipsis can use it.
Supporting this attribute for non-variadic functions(functions without a
C-style ellipsis) gives nice convenience especially when writing code in
C++, we can use it for C++ variadic template functions like this

template<Args...args>
__attribute__((format(printf, 1, 2))) void myPrint (const char * fmt,
Args...args)

The main benefit of variadic functions templates over C vararg
functions is that they make use of the type system for type safety.
I'm not sure I see applying attribute format to them as a very
compelling use case.  (I'd expect the format string in a variadic
function template to use generic conversion specifiers, say %@ or
some such, and only let the caller specify things like flags, width
and precision but not type conversion specifiers).  Is there one
where relying on the type system isn't good enough?

This patch will introduce these changes:
1. It is no longer an error simply to have a function with the format
attribute but no C-style variadic arguments

I'm a little on the fence about this.  On the one hand it seems
unexpected to apply format checking to ordinary (non-variadic)
functions.  On the other, I can't think of anything wrong with it
and it even seems like it could be useful to specify a format for
a fixed number of arguments of fixed types.  Do you have an actual
use case for it or did it just fall out of the varaidic template
implementation?

2. Functions are subjected to warnings/errors as before, except errors
mentioned in point 1 about not being variadic. For example, when a
non-variadic function has wrong arguments, e.g
__attribute__((format(printf, 1, 1))) or when being type-checked.

Note that behaviours of C-style variadic functions do not change, errors
and warnings are given as before.

This patch does it by:
1.   Relaxing several conditions for format attribute:
      -  Will only use POSARG_ELLIPSIS flag to call `get_constant` when
getting attribute arguments of a variadic function
      -  Relax the check for the last argument of the attribute (will not
require an ellipsis argument)
      -  (Before this patch) After passing the above check, current gcc will
call `get_constant` to get the function parameter that the third attribute
argument is pointing to. If POSARG_ELLIPSIS is set, `get_constant` will
look for `...`. If not, `get_constant` will look for a C-style string. Note
that POSARG_ELLIPSIS is set automatically for getting the third attribute
argument.
         (After this patch) POSARG_ELLIPSIS is set only when the function
has C-style '...'. Now, if POSARG_ELLIPSIS is not set, `get_constant` will
not check whether the third argument of format attribute points to a
C-style string.
2.   Modifying expected outcome of a testcase in objc testsuite, where we
expect a warning instead of an error
3.   Adding 2 test files

Successully bootstrapped and regression tested on x86_64-pc-linux-gnu.

Signed-off-by: Le Quang Tuan <lequangtua...@gmail.com>

gcc/c-family/ChangeLog:

* c-attribs.c (positional_argument): allow third argument of format
attribute to point to parameters of any type if the function is not C-style
variadic
* c-format.c (decode_format_attr): read third argument with POSARG_ELLIPSIS
only if the function has has a variable argument
(handle_format_attribute): relax explicit checks for non-variadic functions

gcc/testsuite/ChangeLog:

* gcc.dg/format/attr-3.c: modify comment
* objc.dg/attributes/method-format-1.m: errors do not hold anymore, a
warning is given instead
* g++.dg/warn/format9.C: New test with usage of variadic templates.
* gcc.dg/format/attr-9.c: New test.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 6bf492afcc0..7a17ce671de 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -714,6 +714,11 @@ positional_argument (const_tree fntype, const_tree
atname, tree pos,
    return NULL_TREE;
   }

+  /* For format attribute with argno >= 3, we don't expect any type
+   */
+  if (argno >= 3 && strcmp (IDENTIFIER_POINTER(atname), "format") == 0 &&
!(flags & POSARG_ELLIPSIS ) )
+    return pos;

Hardcoding knowledge of individual attributes in this function doesn't
seem very robust.  Avoiding that is the purpose of the flags argument.
I'd suggest adding a bit to the posargflags enum.

Also, at this point, (flags & POSARG_ELLIPSIS) should be zero as
a result of the test above (not shown) so repeating the test shouldn't
be necessary.

Martin

Reply via email to