diff mbox

Do not warn about C++ compatibility of casts to incomplete type

Message ID 1288367432-3422-1-git-send-email-bonzini@gnu.org
State New
Headers show

Commit Message

Paolo Bonzini Oct. 29, 2010, 3:50 p.m. UTC 
While working on an unrelated patch I noticed a small imprecision
in -Wc++-compat: casts to pointer to incomplete type are erroneously
flagged as invalid in C++, if the occurrence of the incomplete type
in the cast is also the first one in the translation unit.

This patch fixes this imprecision.  Adding a new bit means that
not putting the entire spec.kind into struct c_declspecs is not
cheaper anymore, so this patch does that as well.

Bootstrapped/regtested x86_64-pc-linux-gnu, ok?

2010-10-29  Paolo Bonzini  <bonzini@gnu.org>

        * c-tree.h (enum c_typespec_kind): Add ctsk_none.
        (struct c_declspecs): Replace tagdef_seen_p and type_seen_p
        with typespec_kind.
        * c-decl.c (build_null_declspecs): Initialize typespec_kind.
        (shadow_tag_warned, check_compound_literal_type): Adjust
        uses of tag_defined_p.
        (declspecs_add_type): Set typespec_kind.
        * c-parser.c (c_parser_declaration_or_fndef,
        c_parser_declspecs, c_parser_struct_declaration,
        c_parser_parameter_declaration, c_parser_type_name,
        c_parser_objc_diagnose_bad_element_prefix): Adjust uses
        of type_seen_p.
        * c-typeck.c (c_cast_expr): Use typespec_kind instead of
        tag_defined_p, pass ctsk_firstref through.

testsuite:
2010-10-29  Paolo Bonzini  <bonzini@gnu.org>

        * Wcxx-compat-8.c: Add testcases involving incomplete types.

Comments

Gabriel Dos Reis Oct. 29, 2010, 4 p.m. UTC | #1 
On Fri, Oct 29, 2010 at 10:50 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> While working on an unrelated patch I noticed a small imprecision
> in -Wc++-compat: casts to pointer to incomplete type are erroneously
> flagged as invalid in C++, if the occurrence of the incomplete type
> in the cast is also the first one in the translation unit.
>
> This patch fixes this imprecision.  Adding a new bit means that
> not putting the entire spec.kind into struct c_declspecs is not
> cheaper anymore, so this patch does that as well.

Hi Paolo,

Hmm, I do not understand the purpose of the warning in the first
place.  I do not think there is any difference between C and C++
in this respect, so we should NOT be issueing the warning,

-- Gaby
Paolo Bonzini Oct. 29, 2010, 4:05 p.m. UTC | #2 
On 10/29/2010 06:00 PM, Gabriel Dos Reis wrote:
> On Fri, Oct 29, 2010 at 10:50 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>> While working on an unrelated patch I noticed a small imprecision
>> in -Wc++-compat: casts to pointer to incomplete type are erroneously
>> flagged as invalid in C++, if the occurrence of the incomplete type
>> in the cast is also the first one in the translation unit.
>>
>> This patch fixes this imprecision.  Adding a new bit means that
>> not putting the entire spec.kind into struct c_declspecs is not
>> cheaper anymore, so this patch does that as well.
>
> Hmm, I do not understand the purpose of the warning in the first
> place.  I do not think there is any difference between C and C++
> in this respect, so we should NOT be issueing the warning,

void f(void)
{
         (struct s { int i; } *) 0;
}

$ gcc f.c -fsyntax-only && echo ok
ok
$ g++ f.c -fsyntax-only && echo ok
f.c: In function ‘void f()’:
f.c:3: error: types may not be defined in casts

Paolo
Gabriel Dos Reis Oct. 29, 2010, 4:16 p.m. UTC | #3 
On Fri, Oct 29, 2010 at 11:05 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 10/29/2010 06:00 PM, Gabriel Dos Reis wrote:
>>
>> On Fri, Oct 29, 2010 at 10:50 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>>>
>>> While working on an unrelated patch I noticed a small imprecision
>>> in -Wc++-compat: casts to pointer to incomplete type are erroneously
>>> flagged as invalid in C++, if the occurrence of the incomplete type
>>> in the cast is also the first one in the translation unit.
>>>
>>> This patch fixes this imprecision.  Adding a new bit means that
>>> not putting the entire spec.kind into struct c_declspecs is not
>>> cheaper anymore, so this patch does that as well.
>>
>> Hmm, I do not understand the purpose of the warning in the first
>> place.  I do not think there is any difference between C and C++
>> in this respect, so we should NOT be issueing the warning,
>
> void f(void)
> {
>        (struct s { int i; } *) 0;
> }
>
> $ gcc f.c -fsyntax-only && echo ok
> ok
> $ g++ f.c -fsyntax-only && echo ok
> f.c: In function ‘void f()’:
> f.c:3: error: types may not be defined in casts

Ah yes, there is a different thing, but I would not describe it as

   casts to pointer to incomplete type are erroneously flagged as
invalid in C++,
   if the occurrence of the incomplete type n the cast is also the first one in
   the translation unit.

:-)

that got me confused.  I believe the same situation is also true for sizeof,
and by extension alignof.

-- Gaby
Paolo Bonzini Oct. 29, 2010, 4:45 p.m. UTC | #4 
On 10/29/2010 06:16 PM, Gabriel Dos Reis wrote:
>> void f(void)
>> {
>>        (struct s { int i; } *) 0;
>> }
>>
>> $ gcc f.c -fsyntax-only && echo ok
>> ok
>> $ g++ f.c -fsyntax-only && echo ok
>> f.c: In function ‘void f()’:
>> f.c:3: error: types may not be defined in casts
>
> Ah yes, there is a different thing, but I would not describe it as
>
>     casts to pointer to incomplete type are erroneously flagged as
>     invalid in C++, if the occurrence of the incomplete type n the
>     cast is also the first one in the translation unit.

You quoted the case which I _removed_, while my example above is the 
case which I kept. :)

If you have

void f(void)
{
        (struct s *) 0;
        (struct s *) 0;
}

g++ doesn't give the error (correctly IMO), while -Wc++-compat warns for 
the first line and not for the second.

For sizeof the code paths are completely different and

void f(void)
{
	sizeof (struct s *);
}

does not warn already.

Paolo
Ian Lance Taylor Oct. 29, 2010, 6:06 p.m. UTC | #5 
Paolo Bonzini <bonzini@gnu.org> writes:

> While working on an unrelated patch I noticed a small imprecision
> in -Wc++-compat: casts to pointer to incomplete type are erroneously
> flagged as invalid in C++, if the occurrence of the incomplete type
> in the cast is also the first one in the translation unit.
>
> This patch fixes this imprecision.  Adding a new bit means that
> not putting the entire spec.kind into struct c_declspecs is not
> cheaper anymore, so this patch does that as well.
>
> Bootstrapped/regtested x86_64-pc-linux-gnu, ok?
>
> 2010-10-29  Paolo Bonzini  <bonzini@gnu.org>
>
>         * c-tree.h (enum c_typespec_kind): Add ctsk_none.
>         (struct c_declspecs): Replace tagdef_seen_p and type_seen_p
>         with typespec_kind.
>         * c-decl.c (build_null_declspecs): Initialize typespec_kind.
>         (shadow_tag_warned, check_compound_literal_type): Adjust
>         uses of tag_defined_p.
>         (declspecs_add_type): Set typespec_kind.
>         * c-parser.c (c_parser_declaration_or_fndef,
>         c_parser_declspecs, c_parser_struct_declaration,
>         c_parser_parameter_declaration, c_parser_type_name,
>         c_parser_objc_diagnose_bad_element_prefix): Adjust uses
>         of type_seen_p.
>         * c-typeck.c (c_cast_expr): Use typespec_kind instead of
>         tag_defined_p, pass ctsk_firstref through.
>
> testsuite:
> 2010-10-29  Paolo Bonzini  <bonzini@gnu.org>
>
>         * Wcxx-compat-8.c: Add testcases involving incomplete types.


The patch looks fine to me but I'll leave it to the C frontend
maintainers to approve.

Thanks.

Ian
Joseph Myers Nov. 5, 2010, 5:32 p.m. UTC | #6 
On Fri, 29 Oct 2010, Paolo Bonzini wrote:

> While working on an unrelated patch I noticed a small imprecision
> in -Wc++-compat: casts to pointer to incomplete type are erroneously
> flagged as invalid in C++, if the occurrence of the incomplete type
> in the cast is also the first one in the translation unit.
> 
> This patch fixes this imprecision.  Adding a new bit means that
> not putting the entire spec.kind into struct c_declspecs is not
> cheaper anymore, so this patch does that as well.
> 
> Bootstrapped/regtested x86_64-pc-linux-gnu, ok?

OK.
diff mbox

Patch

Index: gcc/testsuite/gcc.dg/Wcxx-compat-8.c
===================================================================
--- gcc/testsuite/gcc.dg/Wcxx-compat-8.c	(branch diag-2)
+++ gcc/testsuite/gcc.dg/Wcxx-compat-8.c	(working copy)
@@ -22,7 +22,17 @@  enum e2
 struct s3 v3;
 int v4 = C;
 
+enum e3
+{
+  F = sizeof (struct t3),	/* { dg-bogus "invalid in C\[+\]\[+\]" } */
+  /* { dg-error "invalid application of 'sizeof'" "" { target *-*-* } 27 } */
+  G = __alignof__ (struct t4), /* { dg-bogus "invalid in C\[+\]\[+\]" } */
+  /* { dg-error "invalid application of '__alignof__'" "" { target *-*-* } 29 } */
+  H
+};
+
 __typeof__ (struct s5 { int i; }) v5; /* { dg-warning "invalid in C\[+\]\[+\]" } */
+__typeof__ (struct t5) w5; /* { dg-bogus "invalid in C\[+\]\[+\]" } */
 
 int
 f1 (struct s1 *p)
@@ -30,14 +40,26 @@  f1 (struct s1 *p)
   return ((struct s6 { int j; } *) p)->j;  /* { dg-warning "invalid in C\[+\]\[+\]" } */
 }
 
-int
+void *
 f2 (struct s1 *p)
 {
+  return ((struct t6 *) p);  /* { dg-bogus "invalid in C\[+\]\[+\]" } */
+}
+
+int
+f3 (struct s1 *p)
+{
   return (__extension__ (struct s7 { int j; } *)p)->j;
 }
 
 int
-f3 ()
+f4 ()
 {
   return (struct s8 { int i; }) { 0 }.i;  /* { dg-warning "invalid in C\[+\]\[+\]" } */
 }
+
+void *
+f5 ()
+{
+  return &((struct t8) { });  /* { dg-warning "invalid in C\[+\]\[+\]" } */
+}
Index: gcc/c-decl.c
===================================================================
--- gcc/c-decl.c	(branch diag-2)
+++ gcc/c-decl.c	(working copy)
@@ -3634,7 +3634,8 @@  shadow_tag_warned (const struct c_declsp
 		  warned = 1;
 		}
 	    }
-	  else if (!declspecs->tag_defined_p
+	  else if (declspecs->typespec_kind != ctsk_tagdef
+                   && declspecs->typespec_kind != ctsk_tagfirstref
 		   && declspecs->storage_class != csc_none)
 	    {
 	      if (warned != 1)
@@ -3644,7 +3645,8 @@  shadow_tag_warned (const struct c_declsp
 	      warned = 1;
 	      pending_xref_error ();
 	    }
-	  else if (!declspecs->tag_defined_p
+	  else if (declspecs->typespec_kind != ctsk_tagdef
+                   && declspecs->typespec_kind != ctsk_tagfirstref
 		   && (declspecs->const_p
 		       || declspecs->volatile_p
 		       || declspecs->restrict_p
@@ -4580,7 +4582,9 @@  build_compound_literal (location_t loc, 
 void
 check_compound_literal_type (location_t loc, struct c_type_name *type_name)
 {
-  if (warn_cxx_compat && type_name->specs->tag_defined_p)
+  if (warn_cxx_compat
+      && (type_name->specs->typespec_kind == ctsk_tagdef
+          || type_name->specs->typespec_kind == ctsk_tagfirstref))
     warning_at (loc, OPT_Wc___compat,
 		"defining a type in a compound literal is invalid in C++");
 }
@@ -8613,10 +8617,9 @@  build_null_declspecs (void)
   ret->storage_class = csc_none;
   ret->expr_const_operands = true;
   ret->declspecs_seen_p = false;
-  ret->type_seen_p = false;
+  ret->typespec_kind = ctsk_none;
   ret->non_sc_seen_p = false;
   ret->typedef_p = false;
-  ret->tag_defined_p = false;
   ret->explicit_signed_p = false;
   ret->deprecated_p = false;
   ret->default_int_p = false;
@@ -8700,7 +8703,7 @@  declspecs_add_type (location_t loc, stru
   tree type = spec.spec;
   specs->non_sc_seen_p = true;
   specs->declspecs_seen_p = true;
-  specs->type_seen_p = true;
+  specs->typespec_kind = spec.kind;
   if (TREE_DEPRECATED (type))
     specs->deprecated_p = true;
 
@@ -9303,8 +9306,6 @@  declspecs_add_type (location_t loc, stru
     }
   else if (TREE_CODE (type) != ERROR_MARK)
     {
-      if (spec.kind == ctsk_tagdef || spec.kind == ctsk_tagfirstref)
-	specs->tag_defined_p = true;
       if (spec.kind == ctsk_typeof)
 	{
 	  specs->typedef_p = true;
Index: gcc/c-tree.h
===================================================================
--- gcc/c-tree.h	(branch diag-2)
+++ gcc/c-tree.h	(working copy)
@@ -134,6 +134,8 @@  struct c_expr
    only used to distinguish tag definitions, tag references and typeof
    uses.  */
 enum c_typespec_kind {
+  /* No typespec.  This appears only in struct c_declspec.  */
+  ctsk_none,
   /* A reserved keyword type specifier.  */
   ctsk_resword,
   /* A reference to a tag, previously declared, such as "struct foo".
@@ -225,13 +227,14 @@  struct c_declspecs {
   /* Any type specifier keyword used such as "int", not reflecting
      modifiers such as "short", or cts_none if none.  */
   ENUM_BITFIELD (c_typespec_keyword) typespec_word : 8;
+  /* The kind of type specifier if one has been seen, ctsk_none
+     otherwise.  */
+  ENUM_BITFIELD (c_typespec_kind) typespec_kind : 3;
   /* Whether any expressions in typeof specifiers may appear in
      constant expressions.  */
   BOOL_BITFIELD expr_const_operands : 1;
   /* Whether any declaration specifiers have been seen at all.  */
   BOOL_BITFIELD declspecs_seen_p : 1;
-  /* Whether a type specifier has been seen.  */
-  BOOL_BITFIELD type_seen_p : 1;
   /* Whether something other than a storage class specifier or
      attribute has been seen.  This is used to warn for the
      obsolescent usage of storage class specifiers other than at the
@@ -241,10 +244,6 @@  struct c_declspecs {
   BOOL_BITFIELD non_sc_seen_p : 1;
   /* Whether the type is specified by a typedef or typeof name.  */
   BOOL_BITFIELD typedef_p : 1;
-  /* Whether a struct, union or enum type either had its content
-     defined by a type specifier in the list or was the first visible
-     declaration of its tag.  */
-  BOOL_BITFIELD tag_defined_p : 1;
   /* Whether the type is explicitly "signed" or specified by a typedef
      whose type is explicitly "signed".  */
   BOOL_BITFIELD explicit_signed_p : 1;
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(branch diag-2)
+++ gcc/c-parser.c	(working copy)
@@ -1495,7 +1495,8 @@  c_parser_declaration_or_fndef (c_parser 
 	 should diagnose if there were no declaration specifiers) or a
 	 function definition (in which case the diagnostic for
 	 implicit int suffices).  */
-      declarator = c_parser_declarator (parser, specs->type_seen_p,
+      declarator = c_parser_declarator (parser, 
+					specs->typespec_kind != ctsk_none,
 					C_DTR_NORMAL, &dummy);
       if (declarator == NULL)
 	{
@@ -2446,7 +2447,7 @@  c_parser_struct_declaration (c_parser *p
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
     {
       tree ret;
-      if (!specs->type_seen_p)
+      if (specs->typespec_kind == ctsk_none)
 	{
 	  pedwarn (decl_loc, OPT_pedantic,
 		   "ISO C forbids member declarations with no members");
@@ -2496,7 +2497,8 @@  c_parser_struct_declaration (c_parser *p
       if (c_parser_next_token_is (parser, CPP_COLON))
 	declarator = build_id_declarator (NULL_TREE);
       else
-	declarator = c_parser_declarator (parser, specs->type_seen_p,
+	declarator = c_parser_declarator (parser,
+					  specs->typespec_kind != ctsk_none,
 					  C_DTR_NORMAL, &dummy);
       if (declarator == NULL)
 	{
@@ -3140,7 +3142,8 @@  c_parser_parameter_declaration (c_parser
   pending_xref_error ();
   prefix_attrs = specs->attrs;
   specs->attrs = NULL_TREE;
-  declarator = c_parser_declarator (parser, specs->type_seen_p,
+  declarator = c_parser_declarator (parser,
+				    specs->typespec_kind != ctsk_none,
 				    C_DTR_PARM, &dummy);
   if (declarator == NULL)
     {
@@ -3429,7 +3432,8 @@  c_parser_type_name (c_parser *parser)
     }
   pending_xref_error ();
   finish_declspecs (specs);
-  declarator = c_parser_declarator (parser, specs->type_seen_p,
+  declarator = c_parser_declarator (parser,
+				    specs->typespec_kind != ctsk_none,
 				    C_DTR_ABSTRACT, &dummy);
   if (declarator == NULL)
     return NULL;
@@ -7648,7 +7652,8 @@  static bool
 c_parser_objc_diagnose_bad_element_prefix (c_parser *parser, 
 					   struct c_declspecs *specs)
 {
-  if (!specs->declspecs_seen_p || specs->type_seen_p || specs->non_sc_seen_p)
+  if (!specs->declspecs_seen_p || specs->non_sc_seen_p
+      || specs->typespec_kind != ctsk_none)
     {
       c_parser_error (parser, 
       		      "no type or storage class may be specified here,");
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c	(branch diag-2)
+++ gcc/c-typeck.c	(working copy)
@@ -4830,8 +4830,9 @@  c_cast_expr (location_t loc, struct c_ty
   if (CAN_HAVE_LOCATION_P (ret) && !EXPR_HAS_LOCATION (ret))
     SET_EXPR_LOCATION (ret, loc);
 
-  /* C++ does not permits types to be defined in a cast.  */
-  if (warn_cxx_compat && type_name->specs->tag_defined_p)
+  /* C++ does not permits types to be defined in a cast, but it
+     allows references to incomplete types.  */
+  if (warn_cxx_compat && type_name->specs->typespec_kind == ctsk_tagdef)
     warning_at (loc, OPT_Wc___compat,
 		"defining a type in a cast is invalid in C++");