diff mbox

[C] Fix PR40563

Message ID 4C52A23B.6010408@oracle.com
State New
Headers show

Commit Message

Shujing Zhao July 30, 2010, 9:58 a.m. UTC 
On 07/28/2010 09:15 PM, Fabien Chêne wrote:
> 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.
> 
OK. Clear the comment of diagnose_uninitialized_cst_member. 
C_TYPE_FIELDS_READONLY is used before recursing the struct or union.
Uninitialized array of structures would be diagnosed if one of members is const.
The warned type is changed to "strip_array_types (TREE_TYPE (decl)", so that it 
would warn struct or union type, not the array.

The testcase is fixed to test the recursing and test the array of structure that 
has const member.

Tested on trunk and bootstraped.
Is it ok?

Pearly
/gcc
2010-07-30  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-30  Shujing Zhao  <pearly.zhao@oracle.com>
	PR c/40563
	* gcc.dg/Wcxx-compat-20.c: New test

Comments

Shujing Zhao Aug. 11, 2010, 2:01 a.m. UTC | #1 
On 07/30/2010 05:58 PM, Shujing Zhao wrote:
> OK. Clear the comment of diagnose_uninitialized_cst_member. 
> C_TYPE_FIELDS_READONLY is used before recursing the struct or union.
> Uninitialized array of structures would be diagnosed if one of members 
> is const.
> The warned type is changed to "strip_array_types (TREE_TYPE (decl)", so 
> that it would warn struct or union type, not the array.
> 
> The testcase is fixed to test the recursing and test the array of 
> structure that has const member.
> 
> Tested on trunk and bootstraped.
> Is it ok?

ping.

Thanks
Pearly
Joseph Myers Aug. 13, 2010, 5:02 p.m. UTC | #2 
On Fri, 30 Jul 2010, Shujing Zhao wrote:

> OK. Clear the comment of diagnose_uninitialized_cst_member.
> C_TYPE_FIELDS_READONLY is used before recursing the struct or union.
> Uninitialized array of structures would be diagnosed if one of members is
> const.
> The warned type is changed to "strip_array_types (TREE_TYPE (decl)", so that
> it would warn struct or union type, not the array.
> 
> The testcase is fixed to test the recursing and test the array of structure
> that has const member.
> 
> Tested on trunk and bootstraped.
> Is it ok?

This patch is OK.
Shujing Zhao Aug. 17, 2010, 8:31 a.m. UTC | #3 
On 08/14/2010 01:02 AM, Joseph S. Myers wrote:
> On Fri, 30 Jul 2010, Shujing Zhao wrote:
> 
>> OK. Clear the comment of diagnose_uninitialized_cst_member.
>> C_TYPE_FIELDS_READONLY is used before recursing the struct or union.
>> Uninitialized array of structures would be diagnosed if one of members is
>> const.
>> The warned type is changed to "strip_array_types (TREE_TYPE (decl)", so that
>> it would warn struct or union type, not the array.
>>
>> The testcase is fixed to test the recursing and test the array of structure
>> that has const member.
>>
>> Tested on trunk and bootstraped.
>> Is it ok?
> 
> This patch is OK.
> 
Retested on current trunk and committed revision 163296. Thanks

Pearly
diff mbox

Patch

Index: c-decl.c
===================================================================
--- c-decl.c	(revision 162706)
+++ c-decl.c	(working copy)
@@ -4103,6 +4103,35 @@  start_decl (struct c_declarator *declara
   return tem;
 }
 
+/* Subroutine of finish_decl. TYPE is the type of an uninitialized object
+   DECL or the non-array element type if DECL is an uninitialized array.
+   If that type has a const member, diagnose this. */
+
+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++",
+		      strip_array_types (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,18 @@  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);
+    {
+      type = strip_array_types (type);
+      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)
+	       && C_TYPE_FIELDS_READONLY (type))
+	diagnose_uninitialized_cst_member (decl, type);
+    }
 }
 
 /* Given a parsed parameter declaration, decode it into a PARM_DECL.  */
@@ -6859,9 +6895,7 @@  finish_struct (location_t loc, tree t, t
       else
 	{
 	  /* A field that is pseudo-const makes the structure likewise.  */
-	  tree t1 = TREE_TYPE (x);
-	  while (TREE_CODE (t1) == ARRAY_TYPE)
-	    t1 = TREE_TYPE (t1);
+	  tree t1 = strip_array_types (TREE_TYPE (x));
 	  if ((TREE_CODE (t1) == RECORD_TYPE || TREE_CODE (t1) == UNION_TYPE)
 	      && C_TYPE_FIELDS_READONLY (t1))
 	    C_TYPE_FIELDS_READONLY (t) = 1;
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,15 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wc++-compat" } */
+typedef struct s { const int i; } s; /* { dg-message "should be initialized" } */
+union u {const int a; double b;}; /* { dg-message "should be initialized" } */
+struct ts { int a; s v;};
+struct ta { int a; s v[2];};
+
+void f ()
+{
+  s v1; /* { dg-warning "uninitialized const member in" } */
+  s va[2]; /* { dg-warning "uninitialized const member in" } */
+  union u v2; /* { dg-warning "uninitialized const member in" } */ 
+  struct ts v3; /* { dg-warning "uninitialized const member in" } */
+  struct ta ta[2]; /* { dg-warning "uninitialized const member in" } */
+}