Patchwork [C] Fix PR40563

login
register
mail settings
Submitter Shujing Zhao
Date July 21, 2010, 3:09 a.m.
Message ID <4C4664DB.6040902@oracle.com>
Download mbox | patch
Permalink /patch/59400/
State New
Headers show

Comments

Shujing Zhao - July 21, 2010, 3:09 a.m.
Hi,

This patch diagnose uninitialized const field in struct or union when 
-Wc++-compat enabled.

Tested on i686-pc-linux-gnu with no regression.
ok for trunk?
/gcc
2010-07-21  Shujing Zhao  <pearly.zhao@oracle.com>

	PR c/40563
	* c-decl.c (diagnose_uninitialized_cst_member): New function.
	(finish_decl): Use it to issue a -Wc++-compat warning about
	uninitialized const field in struct or union.
	
/gcc/testsuite
2010-07-21  Shujing Zhao  <pearly.zhao@oracle.com>
	PR c/40563
	* gcc.dg/Wcxx-compat-20.c: New test
Joseph S. Myers - July 27, 2010, 1:58 p.m.
On Wed, 21 Jul 2010, Shujing Zhao wrote:

> Index: c-decl.c
> ===================================================================
> --- c-decl.c	(revision 162361)
> +++ c-decl.c	(working copy)
> @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara
>    return tem;
>  }
>  
> +/* Subroutine of finish_decl. Diagnose uninitialized const member of type TYPE.
> +   DECL is the original declaration. */

This comment is unclear.  What you seem to mean is: TYPE is the type of an 
uninitialized object DECL.  If that object has a const member, diagnose 
this.

Why do you need to recurse down among fields?  Why isn't checking 
C_TYPE_FIELDS_READONLY enough?

What if the original uninitialized object is not a structure but an array 
of structures?  Will you diagnose things in that case?

You don't appear to have any testcases where the recursion is needed for 
diagnosis, or where the member in question is an array, only tests 
directly involving a const scalar element of the structure or union.
Shujing Zhao - July 28, 2010, 8:38 a.m.
On 07/27/2010 09:58 PM, Joseph S. Myers wrote:
> On Wed, 21 Jul 2010, Shujing Zhao wrote:
> 
>> Index: c-decl.c
>> ===================================================================
>> --- c-decl.c	(revision 162361)
>> +++ c-decl.c	(working copy)
>> @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara
>>    return tem;
>>  }
>>  
>> +/* Subroutine of finish_decl. Diagnose uninitialized const member of type TYPE.
>> +   DECL is the original declaration. */
> 
> This comment is unclear.  What you seem to mean is: TYPE is the type of an 
> uninitialized object DECL.  If that object has a const member, diagnose 
> this.
> 
Thanks.
> Why do you need to recurse down among fields?  Why isn't checking 
> C_TYPE_FIELDS_READONLY enough?
> 
It used to notice which field should be initialized, just like the diagnostics 
at c++. If the notice is not very necessary, C_TYPE_FIELDS_READONLY is enough.

> What if the original uninitialized object is not a structure but an array 
> of structures?  Will you diagnose things in that case?

C++ would not error an uninitialized array of structure that has a const member. 
  So I think it doesn't need warn at C?
> 
> You don't appear to have any testcases where the recursion is needed for 
> diagnosis, or where the member in question is an array, only tests 
> directly involving a const scalar element of the structure or union.
> 
Yes, they should be added.

Pearly
Joseph S. Myers - July 28, 2010, 9:49 a.m.
On Wed, 28 Jul 2010, Shujing Zhao wrote:

> > Why do you need to recurse down among fields?  Why isn't checking
> > C_TYPE_FIELDS_READONLY enough?
> > 
> It used to notice which field should be initialized, just like the diagnostics
> at c++. If the notice is not very necessary, C_TYPE_FIELDS_READONLY is enough.

You could always check C_TYPE_FIELDS_READONLY before recursing to find the 
field in question.

> > What if the original uninitialized object is not a structure but an array of
> > structures?  Will you diagnose things in that case?
> 
> C++ would not error an uninitialized array of structure that has a const
> member.  So I think it doesn't need warn at C?

Could you please give references to the relevant sections of the C++ 
standard that explain exactly what is or is not permitted in this regard?
Fabien Chêne - July 28, 2010, 1:15 p.m.
2010/7/28 Shujing Zhao <pearly.zhao@oracle.com>:
> On 07/27/2010 09:58 PM, Joseph S. Myers wrote:
>>
>> On Wed, 21 Jul 2010, Shujing Zhao wrote:
>>
>>> Index: c-decl.c
>>> ===================================================================
>>> --- c-decl.c    (revision 162361)
>>> +++ c-decl.c    (working copy)
>>> @@ -4103,6 +4103,35 @@ start_decl (struct c_declarator *declara
>>>   return tem;
>>>  }
>>>  +/* Subroutine of finish_decl. Diagnose uninitialized const member of
>>> type TYPE.
>>> +   DECL is the original declaration. */
>>
>> This comment is unclear.  What you seem to mean is: TYPE is the type of an
>> uninitialized object DECL.  If that object has a const member, diagnose
>> this.
>>
> Thanks.
>>
>> Why do you need to recurse down among fields?  Why isn't checking
>> C_TYPE_FIELDS_READONLY enough?
>>
> It used to notice which field should be initialized, just like the
> diagnostics at c++. If the notice is not very necessary,
> C_TYPE_FIELDS_READONLY is enough.
>
>> What if the original uninitialized object is not a structure but an array
>> of structures?  Will you diagnose things in that case?
>
> C++ would not error an uninitialized array of structure that has a const
> member.  So I think it doesn't need warn at C?

It seems to be PR c++/43719, which should be fixed on mainline.

Patch

Index: c-decl.c
===================================================================
--- c-decl.c	(revision 162361)
+++ c-decl.c	(working copy)
@@ -4103,6 +4103,35 @@  start_decl (struct c_declarator *declara
   return tem;
 }
 
+/* Subroutine of finish_decl. Diagnose uninitialized const member of type TYPE.
+   DECL is the original declaration. */
+
+static void
+diagnose_uninitialized_cst_member (tree decl, tree type)
+{
+  tree field;
+  for (field = TYPE_FIELDS (type); field; field = TREE_CHAIN (field))
+    {
+      tree field_type;
+      if (TREE_CODE (field) != FIELD_DECL)
+	continue;
+      field_type = strip_array_types (TREE_TYPE (field));	
+
+      if (TYPE_QUALS (field_type) & TYPE_QUAL_CONST)
+        {
+	  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc___compat,
+	  	      "uninitialized const member in %qT is invalid in C++",
+		      TREE_TYPE (decl));
+	  inform (DECL_SOURCE_LOCATION (field), "%qD should be initialized", field);
+	}
+
+      if (TREE_CODE (field_type) == RECORD_TYPE
+	  || TREE_CODE (field_type) == UNION_TYPE)
+	diagnose_uninitialized_cst_member (decl, field_type);
+    }
+}
+
+
 /* Finish processing of a declaration;
    install its initial value.
    If ORIGTYPE is not NULL_TREE, it is the original type of INIT.
@@ -4420,11 +4449,16 @@  finish_decl (tree decl, location_t init_
 
   if (warn_cxx_compat
       && TREE_CODE (decl) == VAR_DECL
-      && TREE_READONLY (decl)
       && !DECL_EXTERNAL (decl)
       && DECL_INITIAL (decl) == NULL_TREE)
-    warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc___compat,
-		"uninitialized const %qD is invalid in C++", decl);
+    {
+      if (TREE_READONLY (decl))
+	warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wc___compat,
+		    "uninitialized const %qD is invalid in C++", decl);
+      else if (TREE_CODE (type) == RECORD_TYPE
+	       || TREE_CODE (type) == UNION_TYPE)
+	diagnose_uninitialized_cst_member (decl, type);
+    }
 }
 
 /* Given a parsed parameter declaration, decode it into a PARM_DECL.  */
Index: testsuite/gcc.dg/Wcxx-compat-20.c
===================================================================
--- testsuite/gcc.dg/Wcxx-compat-20.c	(revision 0)
+++ testsuite/gcc.dg/Wcxx-compat-20.c	(revision 0)
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wc++-compat" } */
+typedef struct s { const int i; } s; /* { dg-message "should be initialized" } */
+void foo () { s v; } /* { dg-warning "uninitialized const member in" } */
+
+union u {const int a; double b;}; /* { dg-message "should be initialized" } */
+void bar () { union u f; } /* { dg-warning "uninitialized const member in" } */