diff mbox

[C] warn for empty struct -Wc++-compat

Message ID CAJXstsDk_9Amb+PLbGzbKLe1MrdFQLCT4yKSP3Xe-tE36KvRwA@mail.gmail.com
State New
Headers show

Commit Message

Prathamesh Kulkarni Nov. 11, 2014, 8:38 p.m. UTC 
On Wed, Nov 12, 2014 at 1:28 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Nov 11, 2014 at 11:27:21PM +0530, Prathamesh Kulkarni wrote:
>> 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 ?
>
> Try something like
> /* { dg-warning "empty struct has size 0 in C|declaration does not declare anything" } */
Thanks, that worked.
>
>> 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)
>
> This line is too long, please indent the last argument properly on
> next line.
>
>>    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++");
>> +     }
>
> Drop the { } around warning_at's.
Done.
>
>> +      else
>> +     gcc_unreachable ();
>> +    }
>> +
>
> I don't think this is needed.
Removed.

Is this version 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

Comments

Marek Polacek Nov. 11, 2014, 9:30 p.m. UTC | #1 
On Wed, Nov 12, 2014 at 02:08:03AM +0530, Prathamesh Kulkarni wrote:
> Is this version okay ?

I have no further comments on this patch, so deferring to Joseph.

Thanks,

	Marek
Joseph Myers Nov. 14, 2014, 9:36 p.m. UTC | #2 
On Wed, 12 Nov 2014, Prathamesh Kulkarni wrote:

> Is this version 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.

OK.
Prathamesh Kulkarni Nov. 14, 2014, 10:26 p.m. UTC | #3 
On Sat, Nov 15, 2014 at 3:06 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 12 Nov 2014, Prathamesh Kulkarni wrote:
>
>> Is this version 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.
>
> OK.
Thanks. I will perform complete regression testing, and post the patch
 with modifications to test cases that
may possibly break due to this warning.

Regards,
Prathamesh
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
diff mbox

Patch

Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 217287)
+++ gcc/c/c-decl.c	(working copy)
@@ -7506,12 +7506,23 @@ 
 /* 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
+	warning_at (record_loc, OPT_Wc___compat,
+		    "empty union has size 0 in C, size 1 in C++");
+    }
+
   /* 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 +7855,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,8 @@ 
+/* { 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" } */
+struct C { struct D {}; int x; }; /* { dg-warning "empty struct has size 0 in C|declaration does not declare anything" } */
+struct E { union F {}; int x; }; /* { dg-warning "empty union has size 0 in C|declaration does not declare anything" } */
+union G { union H {}; int x; }; /* { dg-warning "empty union has size 0 in C|declaration does not declare anything" } */
+union I { struct J {}; int x; }; /* { dg-warning "empty struct has size 0 in C|declaration does not declare anything" } */