On 10/05/2016 05:43 PM, Martin Sebor wrote:
> On 09/23/2016 12:00 PM, Jason Merrill wrote:
>> On Thu, Sep 22, 2016 at 7:39 PM, Martin Sebor <msebor@gmail.com> wrote:
>>> On 09/21/2016 02:43 PM, Jason Merrill wrote:
>>>> On Tue, Sep 20, 2016 at 1:01 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>> On 09/16/2016 12:19 PM, Jason Merrill wrote:
>>>>>> On 09/14/2016 01:03 PM, Martin Sebor wrote:
>>>>>>>
>>>>>>> +      /* Type of the member.  */
>>>>>>> +      tree fldtype = TREE_CODE (fld) == FIELD_DECL ? TREE_TYPE
>>>>>>> (fld) : fld;
>>>>>>
>>>>>> Why set "fldtype" to be a TYPE_DECL rather than its type?
>>>>>
>>>>> I'm not sure I understand the question but (IIRC) the purpose of
>>>>> this code is to detect invalid uses of flexible array members in
>>>>> typedefs such as this:
>>>>>
>>>>>    struct S { typedef struct { int i, a[]; } X2 [2]; };
>>>>>
>>>>> Sadly, it doesn't do anything for
>>>>>
>>>>>    struct X { int i, a[]; };
>>>>>    struct S { typedef struct X X2 [2]; };
>>>>
>>>> The issue is I don't see anything that uses fldtype as a decl, and
>>>> it's strange to have a variable called "*type" that can either be a
>>>> type or a decl, which later code still assumes will be a type.
>>>
>>> I suspect I'm simply looking at it from a different point of view.
>>> The definition
>>>
>>>   tree fldtype = TREE_CODE (fld) == FIELD_DECL ? TREE_TYPE (fld) : fld
>>>
>>> denotes the type of the field if fld is a data member.  Otherwise,
>>> it denotes a type (like a typedef).
>>
>> FLD is always a _DECL.  The result of this assignment is that fldtype
>> refers to either a _TYPE or a _DECL.  This creates a strange
>> asymmetry, and none of the following code seems to depend on it.
>> I would prefer
>>
>> tree fldtype = TREE_TYPE (fld);
>>
>> so that it's always a _TYPE.
>
> I mentioned that this code is important to avoid diagnosing typedefs,
> but the example I showed wasn't the right one.  The examples that do
> depend on it are these two:
>
>   struct S1 {
>     typedef int A [0];
>     A a;
>   };
>
>   struct S2 {
>     typedef struct { int i, a[]; } A1;
>     typedef struct { int i, a[]; } A2;
>   };
>
> The expected (pedantic) warning is:
>
>   warning: zero-size array member ‘S1::a’ in an otherwise empty ‘struct S1’
>
> With the change you're asking we end up with:
>
>   warning: zero-size array member ‘S1::a’ not at end of ‘struct S1’
>
> followed by a bogus
>
>   error: flexible array member ‘S2::A1::a’ not at end of ‘struct S2’
>
> So I still don't understand what you're looking for.  It sounds to
> me like you're asking me to rewrite the subsequent code that uses
> fldtype to use the expression above because you don't like that
> fldtype can be either a _DECL or _TYPE.  But repeating that check
> four times doesn't make sense, so what am I missing?

I'm asking you to clarify the logic.  It seems that your change to 
fldtype affects these two tests:

>           if (eltype == fldtype || TYPE_UNNAMED_P (eltype))

>       if (TREE_CODE (fldtype) != ARRAY_TYPE)

...but this is extremely subtle.  It would be a lot clearer to check fld 
for FIELD_DECL or TYPE_DECL explicitly rather than relying on these 
places that treat fldtype as a type to do the right thing because you've 
obfuscated it.  But I'm tired of going back and forth on this, so here's 
a patch.

And now that I notice it, there seems to be no reason to handle typedefs 
deep in the code for handling fields, it's simpler to handle them up top.