Re: [PATCH] handle expressions in __builtin_has_attribute (PR 88383)

[Martin Sebor <msebor@gmail.com>]

On 12/13/18 12:20 PM, Martin Sebor wrote:
> On 12/13/18 11:59 AM, Jeff Law wrote:
>> On 12/5/18 8:55 PM, Martin Sebor wrote:
>>> The __builtin_has_attribute function fails with an ICE when
>>> its first argument is an expression such as INDIRECT_REF, or
>>> many others.  The code simply assumes it's either a type or
>>> a decl.  The attached patch corrects this oversight.
>>>
>>> While testing the fix I also noticed the C++ front end expects
>>> the first operand to be a unary expression, which causes most
>>> other kinds of expressions to be rejected.  The patch fixes
>>> that as well.
>>>
>>> Finally, while testing the fix even more I realized that
>>> the built-in considers the underlying array itself in ARRAY_REF
>>> expressions rather than its type, which leads to inconsistent
>>> results for overaligned arrays (it's the array itself that's
>>> overaligned, not its elements).  So I fixed that too and
>>> adjusted the one test designed to verify this.
>>>
>>> Tested on x86_64-linux.
>>>
>>> Martin
>>>
>>> gcc-88383.diff
>>>
>>> PR c/88383 - ICE calling __builtin_has_attribute on a reference
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>>     PR c/88383
>>>     * c-attribs.c (validate_attribute): Handle expressions.
>>>     (has_attribute): Handle types referenced by expressions.
>>>     Avoid considering array attributes in ARRAY_REF expressions .
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>     PR c/88383
>>>     * parser.c (cp_parser_has_attribute_expression): Handle assignment
>>>     expressions.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>     PR c/88383
>>>     * c-c++-common/builtin-has-attribute-4.c: Adjust expectations.
>>>     * c-c++-common/builtin-has-attribute-6.c: New test.
>> Well, the high level question here is do we want to support this builtin
>> on expressions at all.  Generally attributes apply to types and decls,
>> not expressions.
>>
>> Clearly we shouldn't fault, but my first inclination is that the
>> attribute applies to types or decls, not expressions.  In that case we
>> should just be issuing an error.
>>
>> I could be convinced otherwise, so if you think we should support
>> passing expressions to this builtin, make your case.
> 
> The support is necessary in order to determine the attributes
> in expressions such as:
> 
>    struct S { __attribute__ ((packed)) int a[32]; };
> 
>    extern struct S s;
> 
>    _Static_assert (__builtin_has_attribute (s.a, packed));

An example involving types might be a better one:

   typedef __attribute__ ((may_alias)) int* BadInt;

   void f (BadInt *p)
   {
     _Static_assert (__builtin_has_attribute (*p, may_alias));
   }

Martin