[PATCH v6] attribs: Implement -Wno-attributes=vendor::attr [PR101940]

Marek Polacek polacek@redhat.com
Tue Nov 9 15:55:45 GMT 2021


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 <jason@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.

We have canonicalize_attr_name which I based my code on.

Marek



More information about the Gcc-patches mailing list