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.
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.
thanks,
