Hi Martin,
Thank you for your quick response.
> 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?
One case we have is wrapping a C API in a C++ one. Hence, the underlying
API is C and we must specify types for format arguments. It means that
generic conversion is not possible.
> Do you have an actual
> use case for it or did it just fall out of the varaidic template
> implementation?
Yes, it falls out of the variadic template. It is the only way that helps
variadic templates use format attribute. And I also feel the same, it might
be useful sometimes.
Also, your suggestion on the code is helpful! I have drafted a new patch
here. I have bootstrapped and regression tested it on x86_64-pc-linux-gnu
Regards,
Tuan
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-common.h (enum posargflags): add flag POSARG_ANY for the third argument
of format attribute
* 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.
* 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..a46d882feba 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -714,6 +714,9 @@ positional_argument (const_tree fntype, const_tree
atname, tree pos,
return NULL_TREE;
}
+ if (flags & POSARG_ANY)
+ return pos;
+
/* Where the expected code is STRING_CST accept any pointer
expected by attribute format (this includes possibly qualified
char pointers and, for targets like Darwin, also pointers to
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be4b29a017b..391cfa685d4 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1462,7 +1462,10 @@ enum posargflags {
POSARG_ZERO = 1,
/* Consider positional attribute argument value valid if it refers
to the ellipsis (i.e., beyond the last typed argument). */
- POSARG_ELLIPSIS = 2
+ POSARG_ELLIPSIS = 2,
+ /* Consider positional attribute argument value valid if it refers
+ to an argument of any type */
+ POSARG_ANY = 4
};
extern tree positional_argument (const_tree, const_tree, tree, tree_code,
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index bda3b18fcd0..0cef3152828 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -380,9 +380,16 @@ decode_format_attr (const_tree fntype, tree atname,
tree args,
else
return false;
+ bool has_variable_arg = !type_argument_type(fntype,
type_num_arguments(fntype) + 1);
+ int extra_flag = 0;
+ if (has_variable_arg)
+ extra_flag = POSARG_ELLIPSIS;
+ else
+ extra_flag = POSARG_ANY;
+
if (tree val = get_constant (fntype, atname, *first_arg_num_expr,
3, &info->first_arg_num,
- (POSARG_ZERO | POSARG_ELLIPSIS), validated_p))
+ (POSARG_ZERO | extra_flag), validated_p))
*first_arg_num_expr = val;
else
return false;
@@ -5193,11 +5200,11 @@ handle_format_attribute (tree *node, tree atname,
tree args,
tree arg_type;
/* Verify that first_arg_num points to the last arg,
- the ... */
+ if the last arg is ... */
FOREACH_FUNCTION_ARGS (type, arg_type, iter)
arg_num++;
- if (arg_num != info.first_arg_num)
+ if (arg_num != info.first_arg_num && !type_argument_type(type, arg_num))
{
if (!(flags & (int) ATTR_FLAG_BUILT_IN))
error ("argument to be formatted is not %<...%>");
diff --git a/gcc/testsuite/g++.dg/warn/format9.C
b/gcc/testsuite/g++.dg/warn/format9.C
new file mode 100644
index 00000000000..39b615859fc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/format9.C
@@ -0,0 +1,16 @@
+// Test format attribute used with variadic templates
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wformat" }
+
+template<typename...Args>
+__attribute__((format(printf, 1, 2))) void fa (const char * fmt,
Args...args)
+{
+ return;
+}
+
+int main()
+{
+ fa ("%d%d", 5, 7);
+ fa ("%d%d%d", 5, 6, 7);
+ fa ("%s%d", 3, 4); // { dg-warning "format" "printf warning" }
+}
diff --git a/gcc/testsuite/gcc.dg/format/attr-3.c
b/gcc/testsuite/gcc.dg/format/attr-3.c
index 1b275c5aba8..64990c43472 100644
--- a/gcc/testsuite/gcc.dg/format/attr-3.c
+++ b/gcc/testsuite/gcc.dg/format/attr-3.c
@@ -55,7 +55,7 @@ extern void fg2 () __attribute__((format(gnu_attr_printf,
1, 1))); /* { dg-error
extern void fg3 () __attribute__((format(gnu_attr_printf, 2, 1))); /* {
dg-error "follows" "bad number order" } */
/* The format string argument must be a string type, and the arguments to
- be formatted must be the "...". */
+ be formatted must be the "..." if the function is variadic */
extern void fh0 (int, ...) __attribute__((format(gnu_attr_printf, 1, 2)));
/* { dg-error ".format. attribute argument 2 value .1. refers to parameter
type .int." "format int string" } */
extern void fh1 (signed char *, ...)
__attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format.
attribute argument 2 value .1. refers to parameter type .signed char \\\*."
"signed char string" } */
extern void fh2 (unsigned char *, ...)
__attribute__((format(gnu_attr_printf, 1, 2))); /* { dg-error ".format.
attribute argument 2 value .1. refers to parameter type .unsigned char
\\\*." "unsigned char string" } */
diff --git a/gcc/testsuite/gcc.dg/format/attr-9.c
b/gcc/testsuite/gcc.dg/format/attr-9.c
new file mode 100644
index 00000000000..3f6788f291c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/format/attr-9.c
@@ -0,0 +1,21 @@
+/* Test for format attributes: test usage of the attribute for
non-variadic functions */
+/* Origin: Tuan Le <lequangtua...@gmail.com> */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99 -Wformat" } */
+
+#define DONT_GNU_PROTOTYPE
+#include "format.h"
+
+/* Usage of the attributes */
+extern void fa (const char *, int, const char *)
__attribute__((format(gnu_attr_printf, 1, 2)));
+extern void fb (const char *, int, int, int)
__attribute__((format(gnu_attr_printf, 1, 3)));
+
+/* Some bad uses */
+extern void fc (const char *, int) __attribute__((format(gnu_attr_printf,
1, 1))); /* { dg-error "format string argument follows the arguments to be
formatted" } */
+
+void
+foo (const char *s, int *p)
+{
+ fa ("%d%s", 5, "hello");
+ fa ("%d %d", 5, "hello"); /* { dg-warning "format" "attribute format
__printf__" } */
+}
diff --git a/gcc/testsuite/objc.dg/attributes/method-format-1.m
b/gcc/testsuite/objc.dg/attributes/method-format-1.m
index d3bb997b2aa..443f9c73120 100644
--- a/gcc/testsuite/objc.dg/attributes/method-format-1.m
+++ b/gcc/testsuite/objc.dg/attributes/method-format-1.m
@@ -20,8 +20,8 @@
- (void) log2: (int)level message: (const char *) my_format, ...
__attribute__ ((format (printf, 2))); /* { dg-error "wrong" } */
+ (void) debug2: (const char *) my_format, ... __attribute__ ((format
(printf))); /* { dg-error "wrong" } */
- (void) debug2: (const char *) my_format, ... __attribute__ ((format
(printf))); /* { dg-error "wrong" } */
-+ (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-error "does not refer to a variable argument list" } */
-- (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-error "does not refer to a variable argument list" } */
++ (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-warning "exceeds the number of function parameters" } */
+- (void) alert: (const char *) my_format __attribute__ ((format (printf,
1, 2))); /* { dg-warning "exceeds the number of function parameters" } */
@end
void test (LogObject *object)
--
2.17.1
Vào Th 4, 30 thg 6, 2021 vào lúc 03:56 Martin Sebor <mse...@gmail.com> đã
viết:
> 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
>
