Re: [Tinycc-devel] Patch to correct using incomplete types in structures/unions.

[Thomas Preud'homme]

Le mercredi 2 octobre 2013 11:10:04 Amine Najahi a écrit :
> 
> You will find the new patch attached. However, it does not handle the
> " such a structure (and any union containing, possibly
> recursively, a member that is such a structure) shall not be a member of
> a structure or an
> element of an array."  part.
> The right way to do this is to tag structures containing flexible arrays
> to avoid them being used in
> arrays and being embedded in other structures.
> gcc and clang give a warning for this if you use the -pedantic switch.
> Do you think it is worth implementing?

For sure it would be better but I think the current patch is already a good 
improvement over the current situation and worth being included in tcc. A 
later commit can detect such case. Besides, I think this must be a quite rare 
situation and will force to add a tag and there aren't that many left.

I have two remarks though. First of all, I'd put the first if in the innermost 
while, since you could have a structure defined like this:

struct foo {
  int a[], b;
};

which would not be correct but would pass your test because there would be 
only one execution of the outermost loop. Since it doesn't cost more to do it 
that way, let's do it.


Then one small nitpick, I prefer the message gcc gives when a flexible array 
is not at the end of a structure:

"flexible array member not at end of struct»

whereas your current patch says "invalid type for $struct_name". The type this 
message is about is not invalid, it just can't be placed after a flexible 
array. So I'd rather go with the exact gcc's message or "field '$field_name1' 
cannot be placed after the flexible array member '$field_name2'".

If you can change this, test that all tests still work then you can commit it.

> 
> Thanks,
> Amine

Thanks a lot for your work on this Amine. I hope you don't mind my small 
criticism, I really appreciate your work.

Best regards,

Thomas

signature.asc
Description: This is a digitally signed message part.