* [patch] c++/2972 warn when ctor-initializer leaves uninitialized data @ 2011-11-07 21:47 Jonathan Wakely 2011-11-07 21:58 ` Jason Merrill 2011-11-07 22:42 ` Gabriel Dos Reis 0 siblings, 2 replies; 10+ messages in thread From: Jonathan Wakely @ 2011-11-07 21:47 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill [-- Attachment #1: Type: text/plain, Size: 1097 bytes --] This is a new version of my -Wmeminit patch, first posted to PR c++/2972. Jason suggested combining the Wmeminit warning with the adjacent Weffc++ one which I agree with. The advice in the Effective C++ book actually says not to leave members uninitialized, rather than saying *all* members must have a mem-initializer (which is just annoying in many cases if the type has a safe default constructor), and my patch provides a better check for that by only warning when data is left uninitialized. Unfortunately this doesn't work very well in C++11 mode, as defaulted constructors don't cause warnings when they should do e.g. struct C { int i; C() = default; }; This doesn't produce the same warning as C() {} even though that's what the defaulted constructor is equivalent to. I'm posting it for comment and in case anyone else has time to work on it. * c-family/c.opt (Wmeminit): Add new option. * c-family/c-opts.c: Include it in Wecpp * cp/init.c: Implement Wmeminit. * doc/invoke.texi: document it. * testsuite/g++.dg/warn/Wmeminit.C: New. [-- Attachment #2: 2972.patch --] [-- Type: text/x-patch, Size: 4608 bytes --] Index: c-family/c.opt =================================================================== --- c-family/c.opt (revision 181096) +++ c-family/c.opt (working copy) @@ -461,6 +461,10 @@ C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning Warn about suspicious declarations of \"main\" +Wmeminit +C++ Var(warn_meminit) Warning +Warn about missing member initializers in constructors which leave data uninitialized + 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 181096) +++ c-family/c-opts.c (working copy) @@ -550,7 +550,14 @@ 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 181096) +++ cp/init.c (working copy) @@ -518,13 +518,27 @@ } } - /* 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 leave data + uninitialized. */ + if (warn_meminit && init == NULL_TREE) + { + tree field = default_init_uninitialized_part (type); + if (field) + { + if (DECL_P (field)) + warning_at (DECL_SOURCE_LOCATION (current_function_decl), + OPT_Wmeminit, + "%qD is not initialized in the member initialization" + " list, so %q+#D is uninitialized", member, field); + else + warning_at (DECL_SOURCE_LOCATION (current_function_decl), + OPT_Wmeminit, + "%qD is not initialized in the member initialization" + " list", member); + } + } + /* Get an lvalue for the data member. */ decl = build_class_member_access_expr (current_class_ref, member, /*access_path=*/NULL_TREE, Index: testsuite/g++.dg/warn/Wmeminit.C =================================================================== --- testsuite/g++.dg/warn/Wmeminit.C (revision 0) +++ testsuite/g++.dg/warn/Wmeminit.C (revision 0) @@ -0,0 +1,72 @@ +// PR c++/2972 +// { dg-do compile } +// { dg-options "-Wmeminit" } + +// Warn when a constructor (user-declared or implicitly-declared) +// leaves members uninitialized. + +struct A +{ + int i; + A() : i() { } // { dg-bogus "uninitialized" } +}; + +struct B +{ + int i; + B() { } // { dg-warning "'B::i' is not initialized" } +}; + +struct C // { dg-bogus "uninitialized" } +{ + int i; +}; + +struct D +{ + C c; + D() : c() { } // { dg-bogus "uninitialized" } +}; + +struct E +{ + int i; // { dg-warning "'F::e' is not initialized" } +}; + +struct F +{ + E e; + F() { } +}; + +struct G +{ + int i; // { dg-warning "'H::g' is not initialized" } +}; + +struct H +{ + G g; + H(const H&) { } +}; + +struct I // { dg-warning "'I::i' is not initialized" } +{ + int i; +}; + +struct J : I +{ + J() { } +}; + +struct K // { dg-warning "'K::i' is not initialized" } +{ + int i; +}; + +struct L : K +{ + L(const L&) { } +}; + Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 181096) +++ doc/invoke.texi (working copy) @@ -2371,6 +2371,22 @@ base class does not have a virtual destructor. This warning is enabled by @option{-Wall}. +@item -Wmeminit @r{(C++ and Objective-C++ only)} +@opindex Wmeminit +@opindex Wno-meminit +Warn about missing member initializers in constructors which leave data +uninitialized. A warning will still be given even if the member is +initialized in the constructor body e.g. + +@smallexample +struct A @{ + int i; + A() @{ i = 0; @} // warning: 'A::i' is not initialized +@}; +@end smallexample + +This warning is enabled if -Weffc++ is specified. + @item -Wnarrowing @r{(C++ and Objective-C++ only)} @opindex Wnarrowing @opindex Wno-narrowing ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-07 21:47 [patch] c++/2972 warn when ctor-initializer leaves uninitialized data Jonathan Wakely @ 2011-11-07 21:58 ` Jason Merrill 2011-11-10 20:17 ` Jonathan Wakely 2011-11-07 22:42 ` Gabriel Dos Reis 1 sibling, 1 reply; 10+ messages in thread From: Jason Merrill @ 2011-11-07 21:58 UTC (permalink / raw) To: Jonathan Wakely; +Cc: gcc-patches 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? Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-07 21:58 ` Jason Merrill @ 2011-11-10 20:17 ` Jonathan Wakely 2011-11-10 20:25 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Jonathan Wakely @ 2011-11-10 20:17 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 423 bytes --] 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. [-- Attachment #2: 2972.patch --] [-- Type: text/x-patch, Size: 4719 bytes --] 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); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-10 20:17 ` Jonathan Wakely @ 2011-11-10 20:25 ` Jason Merrill 2011-11-10 20:28 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Jason Merrill @ 2011-11-10 20:25 UTC (permalink / raw) To: Jonathan Wakely; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-10 20:25 ` Jason Merrill @ 2011-11-10 20:28 ` Jason Merrill 2011-11-11 0:00 ` Jonathan Wakely 0 siblings, 1 reply; 10+ messages in thread From: Jason Merrill @ 2011-11-10 20:28 UTC (permalink / raw) To: Jonathan Wakely; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-10 20:28 ` Jason Merrill @ 2011-11-11 0:00 ` Jonathan Wakely 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Wakely @ 2011-11-11 0:00 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-07 21:47 [patch] c++/2972 warn when ctor-initializer leaves uninitialized data Jonathan Wakely 2011-11-07 21:58 ` Jason Merrill @ 2011-11-07 22:42 ` Gabriel Dos Reis 2011-11-07 22:42 ` Jason Merrill 2011-11-07 23:28 ` Jonathan Wakely 1 sibling, 2 replies; 10+ messages in thread From: Gabriel Dos Reis @ 2011-11-07 22:42 UTC (permalink / raw) To: Jonathan Wakely; +Cc: gcc-patches, Jason Merrill On Mon, Nov 7, 2011 at 3:43 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > This is a new version of my -Wmeminit patch, first posted to PR c++/2972. > > Jason suggested combining the Wmeminit warning with the adjacent > Weffc++ one which I agree with. The advice in the Effective C++ book > actually says not to leave members uninitialized, rather than saying > *all* members must have a mem-initializer (which is just annoying in > many cases if the type has a safe default constructor), and my patch > provides a better check for that by only warning when data is left > uninitialized. > > Unfortunately this doesn't work very well in C++11 mode, as defaulted > constructors don't cause warnings when they should do e.g. > > struct C > { > int i; > C() = default; > }; > > This doesn't produce the same warning as C() {} even though that's > what the defaulted constructor is equivalent to. so the defaulted constructor does not initialize C::i? > > I'm posting it for comment and in case anyone else has time to work on it. > > * c-family/c.opt (Wmeminit): Add new option. > * c-family/c-opts.c: Include it in Wecpp > * cp/init.c: Implement Wmeminit. > * doc/invoke.texi: document it. > > * testsuite/g++.dg/warn/Wmeminit.C: New. > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-07 22:42 ` Gabriel Dos Reis @ 2011-11-07 22:42 ` Jason Merrill 2011-11-08 0:18 ` Gabriel Dos Reis 2011-11-07 23:28 ` Jonathan Wakely 1 sibling, 1 reply; 10+ messages in thread From: Jason Merrill @ 2011-11-07 22:42 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: Jonathan Wakely, gcc-patches On 11/07/2011 05:38 PM, Gabriel Dos Reis wrote: >> struct C >> { >> int i; >> C() = default; >> }; > > so the defaulted constructor does not initialize C::i? No, it doesn't. value-initialization of a C will initialize it, but not default-initialization. Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-07 22:42 ` Jason Merrill @ 2011-11-08 0:18 ` Gabriel Dos Reis 0 siblings, 0 replies; 10+ messages in thread From: Gabriel Dos Reis @ 2011-11-08 0:18 UTC (permalink / raw) To: Jason Merrill; +Cc: Jonathan Wakely, gcc-patches On Mon, Nov 7, 2011 at 4:39 PM, Jason Merrill <jason@redhat.com> wrote: > On 11/07/2011 05:38 PM, Gabriel Dos Reis wrote: >>> >>> struct C >>> { >>> int i; >>> C() = default; >>> }; >> >> so the defaulted constructor does not initialize C::i? > > No, it doesn't. value-initialization of a C will initialize it, but not > default-initialization. > Thanks! (obvously I was stuck with earlier variation of the proposal.) -- Gaby ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] c++/2972 warn when ctor-initializer leaves uninitialized data 2011-11-07 22:42 ` Gabriel Dos Reis 2011-11-07 22:42 ` Jason Merrill @ 2011-11-07 23:28 ` Jonathan Wakely 1 sibling, 0 replies; 10+ messages in thread From: Jonathan Wakely @ 2011-11-07 23:28 UTC (permalink / raw) To: Gabriel Dos Reis; +Cc: gcc-patches, Jason Merrill On 7 November 2011 22:38, Gabriel Dos Reis 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. >> >> struct C >> { >> int i; >> C() = default; >> }; >> >> This doesn't produce the same warning as C() {} even though that's >> what the defaulted constructor is equivalent to. > > so the defaulted constructor does not initialize C::i? [class.ctor] p6 A default constructor that is defaulted and not defined as deleted is implicitly defined when it is odr-used (3.2) to create an object of its class type (1.8) or when it is explicitly defaulted after its first declaration. The implicitly-defined default constructor performs the set of initializations of the class that would be performed by a user-written default constructor for that class with no ctor-initializer (12.6.2) and an empty compound-statement. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-11-10 23:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-11-07 21:47 [patch] c++/2972 warn when ctor-initializer leaves uninitialized data Jonathan Wakely 2011-11-07 21:58 ` Jason Merrill 2011-11-10 20:17 ` Jonathan Wakely 2011-11-10 20:25 ` Jason Merrill 2011-11-10 20:28 ` Jason Merrill 2011-11-11 0:00 ` Jonathan Wakely 2011-11-07 22:42 ` Gabriel Dos Reis 2011-11-07 22:42 ` Jason Merrill 2011-11-08 0:18 ` Gabriel Dos Reis 2011-11-07 23:28 ` Jonathan Wakely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).