On 05/26/2015 05:06 AM, Marek Polacek wrote:
Ping.
On Tue, May 19, 2015 at 04:07:53PM +0200, Marek Polacek wrote:
This PR points out that we output same -Wformat warning twice when using
__attribute__ ((format)). The problem was that attribute_value_equal
(called when processing merge_attributes) got two lists:
"format printf, 1, 2" and "__format__ __printf__, 1, 2", these should be
equal. But since attribute_value_equal uses simple_cst_list_equal when
it sees a TREE_LISTs, it doesn't consider "__printf__" and "printf" as
the same, so it said that the two lists aren't same. That means that the
type then contains two same format attributes and we warn twice.
Fixed by handling the format attribute specially. (The patch doesn't
consider the printf and the gnu_printf archetypes as the same, so we still
might get duplicate warnings when combining printf and gnu_printf.)
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-05-19 Marek Polacek <pola...@redhat.com>
PR c/64223
* tree.c (attribute_value_equal): Handle attribute format.
(cmp_attrib_identifiers): Factor out of lookup_ident_attribute.
* gcc.dg/pr64223-1.c: New test.
* gcc.dg/pr64223-2.c: New test.
diff --git gcc/tree.c gcc/tree.c
index 6297f04..a58ad7b 100644
--- gcc/tree.c
+++ gcc/tree.c
@@ -4871,9 +4871,53 @@ simple_cst_list_equal (const_tree l1, const_tree l2)
return l1 == l2;
}
+/* Compare two identifier nodes representing attributes. Either one may
+ be in prefixed __ATTR__ form. Return true if they are the same, false
+ otherwise. */
I think "wrapped" may be better than "prefixed" above. But it's
obviously a nit. Your call whether or not to change.
+
+ if (attr2_len == attr1_len + 4)
+ {
+ const char *p = IDENTIFIER_POINTER (attr2);
+ const char *q = IDENTIFIER_POINTER (attr1);
+ if (p[0] == '_' && p[1] == '_'
+ && p[attr2_len - 2] == '_' && p[attr2_len - 1] == '_'
+ && strncmp (q, p + 2, attr1_len) == 0)
+ return true;;
+ }
+ else if (attr2_len + 4 == attr1_len)
+ {
+ const char *p = IDENTIFIER_POINTER (attr2);
+ const char *q = IDENTIFIER_POINTER (attr1);
+ if (q[0] == '_' && q[1] == '_'
+ && q[attr1_len - 2] == '_' && q[attr1_len - 1] == '_'
+ && strncmp (q + 2, p, attr2_len) == 0)
+ return true;
+ }
Consider canonicalizing and using std::swap so that the longer
identifier is always in attr1 and the second hunk of code can just go
away. Obviously it's not a huge deal and again, your call whether or
not to pursue this very minor cleanup.
Ok for the trunk as is a patch which makes either or both of the trivial
changes noted above.
Jeff
