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

[Prathamesh Kulkarni <bilbotheelffriend at gmail dot com>]

On Tue, Nov 11, 2014 at 3:35 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Nov 11, 2014 at 03:24:48AM +0530, Prathamesh Kulkarni wrote:
>> * gcc/c/c-decl.c
>>   (warn_cxx_compat_finish_struct): Add new parameter of type location_t.
>>     Warn for empty struct.
>>   (finish_struct): Pass loc to warn_cxx_compat_finish_struct.
>>
>> * gcc/testsuite/gcc.dg/Wcxx-compat-22.c: New test-case.
>
> No gcc/c/ and gcc/testsuite/ prefixes.
>
>> Index: gcc/c/c-decl.c
>> ===================================================================
>> --- gcc/c/c-decl.c    (revision 217287)
>> +++ gcc/c/c-decl.c    (working copy)
>> @@ -7506,12 +7506,15 @@
>>  /* 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 struct has size 0 in C, 1 in C++");
>
> This line is too long.
>
> The patch doesn't seem to handle empty unions.
Thanks, this version handles empty unions.
Is this version okay ?

[gcc/c]
  * c-decl.c (c_struct_parse_info): Add new member code.
      (warn_cxx_compat_finish): Add new parameter record_loc.
          Warn for empty struct and unions.
      (finish_struct): Pass loc to warn_cxx_compat_finish.

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

Thank you,
Prathamesh
>
>         Marek

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;
 };
 
 /* Information for the struct or union currently being parsed, or
@@ -7234,6 +7236,7 @@
   struct_parse_info->struct_types.create (0);
   struct_parse_info->fields.create (0);
   struct_parse_info->typedefs_seen.create (0);
+  struct_parse_info->code = code;
 
   /* FIXME: This will issue a warning for a use of a type defined
      within a statement expr used within sizeof, et. al.  This is not
@@ -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");
+    }
+
   /* 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 +7854,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, 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" } */