Re: reject decl with incomplete struct/union type in check_global_declaration()

[Prathamesh Kulkarni <prathamesh dot kulkarni at linaro dot org>]

On 19 January 2016 at 16:49, Marek Polacek <polacek@redhat.com> wrote:
> Sorry for speaking up late, but I think we could do better with formatting
> in this patch:
>
> On Sat, Jan 16, 2016 at 03:45:22PM +0530, Prathamesh Kulkarni wrote:
>> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
>> index 915376d..d36fc67 100644
>> --- a/gcc/c/c-decl.c
>> +++ b/gcc/c/c-decl.c
>> @@ -4791,6 +4791,13 @@ finish_decl (tree decl, location_t init_loc, tree init,
>>          TREE_TYPE (decl) = error_mark_node;
>>        }
>>
>> +      if ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
>> +       || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE)
>> +       && DECL_SIZE (decl) == 0 && TREE_STATIC (decl))
>
> DECL_SIZE yields a tree, so I'd rather see NULL_TREE instead of 0 here (yeah,
> the enclosing code uses 0s :().  The "&& TREE_STATIC..." should be on its own
> line.
>
>> +     {
>> +       incomplete_record_decls.safe_push (decl);
>> +     }
>> +
>
> Redundant braces.
>
>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>> index a0e0052..3c8a496 100644
>> --- a/gcc/c/c-parser.c
>> +++ b/gcc/c/c-parser.c
>> @@ -59,6 +59,8 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "gimple-expr.h"
>>  #include "context.h"
>>
>> +vec<tree> incomplete_record_decls = vNULL;
>
> This could use a comment.
>
>> +
>> +  for (unsigned i = 0; i < incomplete_record_decls.length (); ++i)
>> +    {
>> +      tree decl = incomplete_record_decls[i];
>> +      if (DECL_SIZE (decl) == 0 && TREE_TYPE (decl) != error_mark_node)
>
> I'd s/0/NULL_TREE/.
Thanks for the review, I have done the suggested changes in this
version of the patch.
Ok for trunk ?

Thanks,
Prathamesh
>
>         Marek

Attachment: patch-4.diff
Description: Text document

Attachment: ChangeLog
Description: Binary data