diff mbox

c++/2972 warn when ctor-initializer leaves uninitialized data

Message ID CAH6eHdTnUedQJyD1bcvuJdnjQzscPpSY+D000jvYw7pQ=yAr0Q@mail.gmail.com
State New
Headers show

Commit Message

Jonathan Wakely Nov. 10, 2011, 7:48 p.m. UTC 
On 7 November 2011 21:47, Jason Merrill wrote:
> On 11/07/2011 04:43 PM, Jonathan Wakely wrote:
>>
>> Unfortunately this doesn't work very well in C++11 mode, as defaulted
>> constructors don't cause warnings when they should do e.g.
>
> Maybe check this in defaulted_late_check?

I tried that (attached) and it does cause warnings in defaulted
constructed, but even for members with an NSDMI, which are not
uninitialized.

Comments

Jason Merrill Nov. 10, 2011, 8:10 p.m. UTC | #1 
On 11/10/2011 02:48 PM, Jonathan Wakely wrote:
> +warn_missing_meminits (tree type, tree cons)
> +{
> +  tree mem_inits = sort_mem_initializers (type, NULL_TREE);
> +  while (mem_inits)
> +    {
> +      tree member = TREE_PURPOSE (mem_inits);
> +      /* TODO do not warn if brace-or-equal-initializer */
> +      warn_meminit_leaves_uninitialized (member, TREE_TYPE (member), cons);
> +      mem_inits = TREE_CHAIN (mem_inits);
> +    }
> +}

Check DECL_INITIAL (member) to tell if it has an NSDMI.

Jason
Jason Merrill Nov. 10, 2011, 8:17 p.m. UTC | #2 
On 11/10/2011 03:10 PM, Jason Merrill wrote:
> On 11/10/2011 02:48 PM, Jonathan Wakely wrote:
>> +warn_missing_meminits (tree type, tree cons)
>> +{
>> + tree mem_inits = sort_mem_initializers (type, NULL_TREE);
>> + while (mem_inits)
>> + {
>> + tree member = TREE_PURPOSE (mem_inits);
>> + /* TODO do not warn if brace-or-equal-initializer */
>> + warn_meminit_leaves_uninitialized (member, TREE_TYPE (member), cons);
>> + mem_inits = TREE_CHAIN (mem_inits);
>> + }
>> +}
>
> Check DECL_INITIAL (member) to tell if it has an NSDMI.

Actually, why not just use default_init_uninitialized_part (type)?

> +  if (warn_meminit && (kind == sfk_constructor || kind == sfk_copy_constructor
> +        || kind == sfk_move_constructor))
> +    warn_missing_meminits (current_class_type, fn);

We only want to do this for sfk_constructor; the others initialize all 
fields.

Jason
Jonathan Wakely Nov. 10, 2011, 11:24 p.m. UTC | #3 
On 10 November 2011 20:17, Jason Merrill wrote:
> On 11/10/2011 03:10 PM, Jason Merrill wrote:
>>
>> On 11/10/2011 02:48 PM, Jonathan Wakely wrote:
>>>
>>> +warn_missing_meminits (tree type, tree cons)
>>> +{
>>> + tree mem_inits = sort_mem_initializers (type, NULL_TREE);
>>> + while (mem_inits)
>>> + {
>>> + tree member = TREE_PURPOSE (mem_inits);
>>> + /* TODO do not warn if brace-or-equal-initializer */
>>> + warn_meminit_leaves_uninitialized (member, TREE_TYPE (member), cons);
>>> + mem_inits = TREE_CHAIN (mem_inits);
>>> + }
>>> +}
>>
>> Check DECL_INITIAL (member) to tell if it has an NSDMI.
>
> Actually, why not just use default_init_uninitialized_part (type)?
>
>> +  if (warn_meminit && (kind == sfk_constructor || kind ==
>> sfk_copy_constructor
>> +        || kind == sfk_move_constructor))
>> +    warn_missing_meminits (current_class_type, fn);
>
> We only want to do this for sfk_constructor; the others initialize all
> fields.

Doh, of course.  Thanks for the pointers, I'll have another stab at
it. I really want to get this warning implemented eventually.
diff mbox

Patch

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 181173)
+++ c-family/c.opt	(working copy)
@@ -461,6 +461,10 @@  Wmain
 C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning
 Warn about suspicious declarations of \"main\"
 
+Wmeminit
+C++ Var(warn_meminit) Warning
+Warn about POD members which are not initialized in a constructor initialization list
+
 Wmissing-braces
 C ObjC C++ ObjC++ Var(warn_missing_braces) Warning
 Warn about possibly missing braces around initializers
Index: c-family/c-opts.c
===================================================================
--- c-family/c-opts.c	(revision 181173)
+++ c-family/c-opts.c	(working copy)
@@ -550,7 +550,14 @@  c_common_handle_option (size_t scode, co
     case OPT_Weffc__:
       warn_ecpp = value;
       if (value)
-        warn_nonvdtor = true;
+        {
+          /* Effective C++ rule 12 says to prefer using a mem-initializer
+             to assignment. */
+          warn_meminit = true;
+          /* Effective C++ rule 14 says to declare destructors virtual
+             in polymorphic classes. */
+          warn_nonvdtor = true;
+        }
       break;
 
     case OPT_ansi:
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 181173)
+++ cp/init.c	(working copy)
@@ -485,6 +485,42 @@  build_value_init_noctor (tree type, tsub
   return build_zero_init (type, NULL_TREE, /*static_storage_p=*/false);
 }
 
+/* Warn if default initialization of MEMBER of type TYPE in constructor
+ * CONS will leave some parts uninitialized.  */
+
+static void
+warn_meminit_leaves_uninitialized (tree member, tree type, tree cons)
+{
+  tree field = default_init_uninitialized_part (type);
+  if (!field)
+    return;
+
+  if (DECL_P (field))
+    warning_at (DECL_SOURCE_LOCATION (cons), OPT_Wmeminit,
+                "no member initializer for %qD so %q+#D is uninitialized",
+                member, field);
+  else
+    warning_at (DECL_SOURCE_LOCATION (cons), OPT_Wmeminit,
+                "no member initializer for %qD so it is uninitialized",
+                member);
+}
+
+/* Warn if defaulted constructor CONS for TYPE with no mem-initializer-list
+   will leave uninitialized data.  */
+
+void
+warn_missing_meminits (tree type, tree cons)
+{
+  tree mem_inits = sort_mem_initializers (type, NULL_TREE);
+  while (mem_inits)
+    {
+      tree member = TREE_PURPOSE (mem_inits);
+      /* TODO do not warn if brace-or-equal-initializer */
+      warn_meminit_leaves_uninitialized (member, TREE_TYPE (member), cons);
+      mem_inits = TREE_CHAIN (mem_inits);
+    }
+}
+
 /* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of
    arguments.  If TREE_LIST is void_type_node, an empty initializer
    list was given; if NULL_TREE no initializer was given.  */
@@ -518,12 +554,10 @@  perform_member_init (tree member, tree i
 	}
     }
 
-  /* Effective C++ rule 12 requires that all data members be
-     initialized.  */
-  if (warn_ecpp && init == NULL_TREE && TREE_CODE (type) != ARRAY_TYPE)
-    warning_at (DECL_SOURCE_LOCATION (current_function_decl), OPT_Weffc__,
-		"%qD should be initialized in the member initialization list",
-		member);
+  /* Warn if there is no initializer for a member which will be left
+     uninitialized.  */
+  if (warn_meminit && init == NULL_TREE)
+      warn_meminit_leaves_uninitialized (member, type, current_function_decl);
 
   /* Get an lvalue for the data member.  */
   decl = build_class_member_access_expr (current_class_ref, member,
Index: cp/method.c
===================================================================
--- cp/method.c	(revision 181173)
+++ cp/method.c	(working copy)
@@ -1655,6 +1655,10 @@  defaulted_late_check (tree fn)
 
   if (DECL_DELETED_FN (implicit_fn))
     DECL_DELETED_FN (fn) = 1;
+
+  if (warn_meminit && (kind == sfk_constructor || kind == sfk_copy_constructor
+        || kind == sfk_move_constructor))
+    warn_missing_meminits (current_class_type, fn);
 }
 
 /* Returns true iff FN can be explicitly defaulted, and gives any
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 181173)
+++ cp/cp-tree.h	(working copy)
@@ -4915,6 +4915,7 @@  extern bool user_provided_p			(tree);
 extern bool type_has_user_provided_constructor  (tree);
 extern bool type_has_user_provided_default_constructor (tree);
 extern tree default_init_uninitialized_part (tree);
+extern void warn_missing_meminits (tree, tree);
 extern bool trivial_default_constructor_is_constexpr (tree);
 extern bool type_has_constexpr_default_constructor (tree);
 extern bool type_has_virtual_destructor		(tree);