| Submitter | Jonathan Wakely |
|---|---|
| Date | Nov. 7, 2011, 9:43 p.m. |
| Message ID | <CAH6eHdRE9192yx6QU1GEQviQ5iSz5zUkNuaH8KJcoojmUDZnDQ@mail.gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/124190/ |
| State | New |
| Headers | show |
Comments
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
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. >
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
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.
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
Patch
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
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.