Re: [PATCH v2][RFC] Canonize names of attributes.

[Jason Merrill <jason at redhat dot com>]

On Fri, Aug 4, 2017 at 9:42 AM, Martin Liška <mliska@suse.cz> wrote:
> On 08/02/2017 01:25 PM, Joseph Myers wrote:
>> On Thu, 13 Jul 2017, Martin Liška wrote:
>>
>>> +/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
>>> +   so that we have a canonical form of attribute names.  */
>>> +
>>> +static inline tree
>>> +canonicalize_attr_name (tree attr_name)
>>> +{
>>> +  const size_t l = IDENTIFIER_LENGTH (attr_name);
>>> +  const char *s = IDENTIFIER_POINTER (attr_name);
>>> +
>>> +  if (l > 4 && s[0] == '_')
>>> +    {
>>> +      gcc_checking_assert (s[l - 2] == '_');
>>> +      return get_identifier_with_length (s + 2, l - 4);
>>> +    }
>>> +
>>> +  return attr_name;
>>
>> For this to (a) be correct, (b) not trigger the assertion, there must be a
>> precondition that attr_name either starts and ends with __, or does not
>> start with _, or has length 4 or less.  I don't see anything in the
>> callers to ensure this precondition holds, so that, for example,
>> __attribute__ ((_foobar)) does not trigger the assertion, and
>> __attribute__ ((_xformat__)) is not wrongly interpreted as a format
>> attribute (and similarly for attribute arguments when those are
>> canonicalized).
>
> Thanks for the review. I've updated to to canonicalize just __.+__.
> I added test-case for that: gcc/testsuite/gcc.dg/Wattributes-5.c.
>
>>
>>> +/* Compare attribute identifiers ATTR1 and ATTR2 with length ATTR1_LEN and
>>> +   ATTR2_LEN.  */
>>> +
>>> +static inline bool
>>> +cmp_attribs (const char *attr1, size_t attr1_len,
>>> +         const char *attr2, size_t attr2_len)
>>> +{
>>> +  gcc_checking_assert (attr1_len == 0 || attr1[0] != '_');
>>> +  gcc_checking_assert (attr2_len == 0 || attr2[0] != '_');
>
> And I removed these asserts.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?

OK.

Jason