Re: [C PATCH] warn for empty struct -Wc++-compat

[Prathamesh Kulkarni <bilbotheelffriend at gmail dot com>]

On Tue, Nov 11, 2014 at 4:30 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Nov 11, 2014 at 04:45:46AM +0530, Prathamesh Kulkarni wrote:
>> Index: gcc/c/c-decl.c
>> ===================================================================
>> --- gcc/c/c-decl.c    (revision 217287)
>> +++ gcc/c/c-decl.c    (working copy)
>> @@ -606,6 +606,8 @@
>>    /* If warn_cxx_compat, a list of typedef names used when defining
>>       fields in this struct.  */
>>    vec<tree> typedefs_seen;
>> +  /* code to distinguish between struct/union */
>> +  enum tree_code code;
>
> I don't think this is desirable, you might just pass T down from
> finish_struct to warn_cxx_compat_finish_struct.
>
>> @@ -7506,12 +7509,19 @@
>>  /* Finish up struct info used by -Wc++-compat.  */
>>
>>  static void
>> -warn_cxx_compat_finish_struct (tree fieldlist)
>> +warn_cxx_compat_finish_struct (tree fieldlist, location_t record_loc)
>>  {
>>    unsigned int ix;
>>    tree x;
>>    struct c_binding *b;
>>
>> +  if (fieldlist == NULL_TREE)
>> +    {
>> +      warning_at (record_loc, OPT_Wc___compat,
>> +               "empty %s has size 0 in C, 1 in C++",
>> +               (struct_parse_info->code == RECORD_TYPE) ? "struct" : "union");
>> +    }
>> +
>
> I think this won't work well wrt translations, so you need to have
> an if here.  See the pedwarns at the beginning of finish_struct.
>
>> Index: gcc/testsuite/gcc.dg/Wcxx-compat-22.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/Wcxx-compat-22.c     (revision 0)
>> +++ gcc/testsuite/gcc.dg/Wcxx-compat-22.c     (working copy)
>> @@ -0,0 +1,4 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-Wc++-compat" } */
>> +struct A {}; /* { dg-warning "empty struct has size 0 in C" } */
>> +union B {}; /* { dg-warning "empty union has size 0 in C" } */
>
> Please also test an empty struct in a struct.
I tried the following:
struct A { struct B {}; int x; } /* { dg-warning "empty struct has
size 0 in C" } */
                                           /* { dg-bogus "delcaration
does not declare anything" } */
but it fails in excess errors.
How do I resolve this ?

Is the rest okay ?
[gcc/c]
  * c-decl.c (warn_cxx_compat_finish_struct): New parameters code, record_loc.
        Warn for empty struct.
    (finish_struct): Pass TREE_CODE (t) and loc to
warn_cxx_compat_finish_struct.

[gcc/testsuite/gcc.dg]
  * Wcxx-compat-22.c: New test-case.

Thank you,
Prathamesh
>
> Thanks,
>
>         Marek

Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 217287)
+++ gcc/c/c-decl.c	(working copy)
@@ -7506,12 +7506,28 @@
 /* Finish up struct info used by -Wc++-compat.  */
 
 static void
-warn_cxx_compat_finish_struct (tree fieldlist)
+warn_cxx_compat_finish_struct (tree fieldlist, enum tree_code code, location_t record_loc)
 {
   unsigned int ix;
   tree x;
   struct c_binding *b;
 
+  if (fieldlist == NULL_TREE)
+    {
+      if (code == RECORD_TYPE)
+	{
+	  warning_at (record_loc, OPT_Wc___compat,
+		      "empty struct has size 0 in C, size 1 in C++");
+	}
+      else if (code == UNION_TYPE)
+	{
+	  warning_at (record_loc, OPT_Wc___compat,
+		      "empty union has size 0 in C, size 1 in C++");
+	}
+      else
+	gcc_unreachable ();
+    }
+
   /* Set the C_TYPE_DEFINED_IN_STRUCT flag for each type defined in
      the current struct.  We do this now at the end of the struct
      because the flag is used to issue visibility warnings, and we
@@ -7844,7 +7860,7 @@
 			  DECL_EXPR, build_decl (loc, TYPE_DECL, NULL, t)));
 
   if (warn_cxx_compat)
-    warn_cxx_compat_finish_struct (fieldlist);
+    warn_cxx_compat_finish_struct (fieldlist, TREE_CODE (t), loc);
 
   struct_parse_info->struct_types.release ();
   struct_parse_info->fields.release ();
Index: gcc/testsuite/gcc.dg/Wcxx-compat-22.c
===================================================================
--- gcc/testsuite/gcc.dg/Wcxx-compat-22.c	(revision 0)
+++ gcc/testsuite/gcc.dg/Wcxx-compat-22.c	(working copy)
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options "-Wc++-compat" } */
+struct A {}; /* { dg-warning "empty struct has size 0 in C" } */
+union B {}; /* { dg-warning "empty union has size 0 in C" } */