PING #2 [PATCH] handle expressions in __builtin_has_attribute (PR 88383)

[Martin Sebor <msebor@gmail.com>]

Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html

On 1/3/19 3:22 PM, Martin Sebor wrote:
> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html
> 
> On 12/20/18 7:51 PM, Martin Sebor wrote:
>> Jeff, did this and the rest of the discussion answer your question
>> and if so, is the patch okay to commit?
>>
>>    https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00337.html
>>
>> Martin
>>
>> On 12/13/18 12:48 PM, Martin Sebor wrote:
>>> 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
>>
>