[PATCH, C] Fix PR40563

[PATCH, C] Fix PR40563

[Shujing Zhao]

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]

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?

[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 358 bytes --]

/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


[-- Attachment #3: pr40563.patch --]
[-- Type: text/x-patch, Size: 2619 bytes --]

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" } */

Re: [PATCH, C] Fix PR40563

[Joseph S. Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH, C] Fix PR40563

[Shujing Zhao]

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

Re: [PATCH, C] Fix PR40563

[Joseph S. Myers]

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?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH, C] Fix PR40563

[Fabien Chêne]

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.

-- 
Fabien

Re: [PATCH, C] Fix PR40563

[Shujing Zhao]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

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


[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 358 bytes --]

/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


[-- Attachment #3: pr40563.patch --]
[-- Type: text/x-patch, Size: 3505 bytes --]

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" } */
+} 

Re: [PATCH, C] Fix PR40563

[Shujing Zhao]

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

Re: [PATCH, C] Fix PR40563

[Joseph S. Myers]

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH, C] Fix PR40563

[Shujing Zhao]

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