On 11/9/21 10:55, Marek Polacek wrote:
On Tue, Nov 09, 2021 at 08:09:10AM +0100, Bernhard Reutner-Fischer wrote:
On Tue, 9 Nov 2021 00:12:10 -0500
Jason Merrill <ja...@redhat.com> wrote:
On 11/8/21 20:41, Marek Polacek wrote:
On Sat, Nov 06, 2021 at 03:29:57PM -0400, Jason Merrill wrote:
+ for (auto opt : v)
+ {
+ char *cln = strstr (opt, "::");
+ /* We don't accept '::attr'. */
+ if (cln == nullptr || cln == opt)
+ {
+ error ("wrong argument to ignored attributes");
+ inform (input_location, "valid format is %<ns::attr%> or %<ns::%>");
+ continue;
+ }
+ char *vendor_start = opt;
+ ptrdiff_t vendor_len = cln - opt;
+ char *attr_start = cln + 2;
+ /* This could really use rawmemchr :(. */
+ ptrdiff_t attr_len = strchr (attr_start, '\0') - attr_start;
+ /* Verify that they look valid. */
+ auto valid_p = [](const char *const s, ptrdiff_t len) {
+ for (int i = 0; i < len; ++i)
+ if (!ISALNUM (*s) && *s != '_')
+ return false;
+ return true;
+ };
+ if (!valid_p (vendor_start, vendor_len)
+ || !valid_p (attr_start, attr_len))
+ {
+ error ("wrong argument to ignored attributes");
+ continue;
+ }
+ /* Turn "__attr__" into "attr" so that we have a canonical form of
+ attribute names. Likewise for vendor. */
+ auto strip = [](char *&s, ptrdiff_t &l) {
+ if (l > 4 && s[0] == '_' && s[1] == '_'
+ && s[l - 1] == '_' && s[l - 2] == '_')
+ {
+ s += 2;
+ l -= 4;
+ }
+ };
+ strip (attr_start, attr_len);
+ strip (vendor_start, vendor_len);
+ /* We perform all this hijinks so that we don't have to copy OPT. */
+ tree vendor_id = get_identifier_with_length (vendor_start, vendor_len);
+ tree attr_id = get_identifier_with_length (attr_start, attr_len);
+ /* If we've already seen this vendor::attr, ignore it. Attempting to
+ register it twice would lead to a crash. */
+ if (lookup_scoped_attribute_spec (vendor_id, attr_id))
+ continue;
Hmm, this looks like it isn't handling the case of previously ignoring
vendor::; it seems we'll look for vendor::<empty string> instead, not
find it, and register again.
Yes, for -Wno-attributes=vendor::,vendor:: we call register_scoped_attributes
twice, but I think that's OK: register_scoped_attributes will see that
find_attribute_namespace finds the namespace and returns without creating a new
one.
I think the current code handles -Wno-attributes=vendor::a,vendor:: well so
I'm not sure if I should change it.
Makes sense. But we should be able to avoid calling
lookup_scoped_attribute_spec, and even get_identifier_with_length
(attr...), if attr_len is 0.
I don't know if there are __attrib with 2 leading underscores but no
trailing that we accept and should normalize?
And it is surprising that we do not have a simple normalize_attr_name().
There must be existing spots where we would normalize attribute names?
extract_attribute_substring does about the same but with a struct substring(?).
Maybe that's just to avoid a normal string hash.
We have canonicalize_attr_name which I based my code on.
We could factor out of that function an overload equivalent to your
strip lambda?
Jason
