If I read clause 12.6.2, 8.5 and 3.9 correctly, all scalar member variables shall be default initialized to 0 unless there is an explicit initialization in the constructor. The attached source demonstrate the problem. The behaviour is the same for gcc 2.95.2 and gcc 3.0 (using the internet test compiler.) A workaround is to initialize all member variables explicitly, adding ': i(), p()' to the ctor in the example. Release: 2.95 -> 3.4 Environment: Debian GNU/Linux 2.2 How-To-Repeat: Compile and watch the assert() fail.
State-Changed-From-To: open->suspended State-Changed-Why: you are incorrect. [12.6.2]/4 specifies that POD types (of which scalar types are), are not initialized. However, we should warn, if the -Wuninitialized flag is given, so I've made this a suspended change-request.
From: nathan@gcc.gnu.org To: gcc-gnats@gcc.gnu.org, nobody@gcc.gnu.org, pere@hungry.com Cc: Subject: Re: c++/2972 Date: 27 May 2001 16:15:03 -0000 Synopsis: Uninitialized member variables are not default initialized State-Changed-From-To: open->suspended State-Changed-By: nathan State-Changed-When: Sun May 27 09:15:03 2001 State-Changed-Why: you are incorrect. [12.6.2]/4 specifies that POD types (of which scalar types are), are not initialized. However, we should warn, if the -Wuninitialized flag is given, so I've made this a suspended change-request. http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view&pr=2972&database=gcc
State-Changed-From-To: suspended->analyzed State-Changed-Why: As far as I can see, this PR is not waiting for the resolution of any issue, so its state should be "analyzed". What Nathan is suggesting is to add a warning for this: -------------------------- struct A { int f,g; A() { f = g; } }; -------------------------- -Wuninitialized could detect the use of g before initialization within the constructor.
Responsible-Changed-From-To: unassigned->gdr Responsible-Changed-Why: Diagnostic mantainer
Also, it should detect any scalar member variables that are not assigned to in any way in the constructor.
(In reply to comment #5) > Also, it should detect any scalar member variables that are not assigned to in > any way in the constructor. Agreed. However -Wunitialized is taken over by the middle-end. This is one more case where I agre with Mark Mitchell that the middl-end should not try those things. Long term, we should move such warnings to the front-end. Yes I know, that is not a universally held opinon.
Still the same with gcc version 4.4.0 20080801 (experimental) [trunk revision 138448]
Related to PR 19808. (In reply to comment #6) > However -Wunitialized is taken over by the middle-end. This is That doesn't mean that you cannot produce uninitialized warnings in the front-end for clear-cut cases like this. In fact, it is impossible to detect this case in the middle-end because of the code produced by the C++ front-end,
I've been experimenting with this patch, which warns if there is a missing mem-initializer for a scalar. It gives a false positive for cases were the member is assigned to in the constructor body, or otherwise initialized before use, but it's a start, and has already helped me find some missing mem-initializers in real code. --- c.opt.orig 2010-06-02 13:47:02.120129255 +0000 +++ c.opt 2010-06-02 13:07:51.944440072 +0000 @@ -304,6 +304,10 @@ C ObjC C++ ObjC++ Var(warn_main) Init(-1) Warning Warn about suspicious declarations of \"main\" +Wmeminit +C++ Var(warn_meminit) Init(-1) 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 --- c-opts.c.orig 2010-06-02 13:46:49.774040694 +0000 +++ c-opts.c 2010-06-02 13:11:57.829233564 +0000 @@ -492,6 +492,10 @@ cpp_opts->warn_invalid_pch = value; break; + case OPT_Wmeminit: + warn_meminit = value; + break; + case OPT_Wmissing_include_dirs: cpp_opts->warn_missing_include_dirs = value; break; --- cp/init.c.orig 2010-06-02 13:46:31.125713124 +0000 +++ cp/init.c 2010-06-02 13:21:01.473640135 +0000 @@ -424,6 +424,12 @@ tree decl; tree type = TREE_TYPE (member); + /* warn if there is no initializer for a POD member */ + if (warn_meminit && init == NULL_TREE && layout_pod_type_p (strip_array_types (type))) + warning_at (DECL_SOURCE_LOCATION (current_function_decl), OPT_Wmeminit, + "%qD is not initialized in the member initialization list", + member); + /* Effective C++ rule 12 requires that all data members be initialized. */ if (warn_ecpp && init == NULL_TREE && TREE_CODE (type) != ARRAY_TYPE)
(In reply to comment #9) > I've been experimenting with this patch, which warns if there is a missing > mem-initializer for a scalar. > > It gives a false positive for cases were the member is assigned to in the > constructor body, or otherwise initialized before use, but it's a start, and > has already helped me find some missing mem-initializers in real code. Nice but I am afraid there may be too many false positives. And what is different between this and the -Weffc++ warning given just below it? > > + case OPT_Wmeminit: > + warn_meminit = value; > + break; > + You do not need this. This is handled automatically when you defined Var in the opt files.
(In reply to comment #10) > (In reply to comment #9) > > I've been experimenting with this patch, which warns if there is a missing > > mem-initializer for a scalar. > > > > It gives a false positive for cases were the member is assigned to in the > > constructor body, or otherwise initialized before use, but it's a start, and > > has already helped me find some missing mem-initializers in real code. > > Nice but I am afraid there may be too many false positives. Yes, I'm not proposing it in that state. It's just an experiment but has been useful for me already. > And what is > different between this and the -Weffc++ warning given just below it? -Weffc++ also enables dozens of other warnings, most of which I don't find useful. It is about good style, not dangerous code, and I don't like the compiler to enforce style. In this specific case, the Weffc++ warning complains if *any* member has no mem-initializer. I only care about POD members which will be uninitialized, I don't want to add a mem-initializer for e.g. a std::string member which has a default constructor. > > + case OPT_Wmeminit: > > + warn_meminit = value; > > + break; > > + > > You do not need this. This is handled automatically when you defined Var in the > opt files. Ah OK, thanks!
Apart from the false positives, another problem is that the check for layout_pod_type_p is not right. An empty class is a POD but doesn't need initialising.
About -Weffc++, we also have a PR (16166) about splitting it... Not that I think we should really do that - adding a dozen of -Weffc++-type warnings - but I believe it would be a good idea to finally resolve one way or the other all those long standing -Weffc++ PRs (we have 5 or 6)... If you ask my opinion, we should probably *not* split the option, and instead reduce *a lot* the false positives for the various warnings, even if that means adding false negatives.
Created attachment 20817 [details] better -Wmeminit patch This version ignores empty classes and checks for a nontrivial default ctor instead of layout_pod_type. This patch doesn't enable the warning unless explicity requested. I realise that this warning is about enforcing style ("members should be initialised in the mem-initializer-list not in the ctor body") but that's ok because it's my preferred style, I just don't want the compiler to enforce other people's preferred style ;)
Created attachment 22623 [details] updated -Wmeminit patch for trunk (4.6.0)
new patch http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01068.html
Jon, are you actively working on this? I found this last message: http://gcc.gnu.org/ml/gcc-patches/2011-11/msg01477.html Let me know if I can help...
No, not at present. I tried using default_init_uninitialized_part but it either missed cases or produce ICEs, and I never solved the problems. I can send you my work-in-progress when I get home, it would be great if you could help me with the issues I don't understand.
Eh, I'm of course not sure that I can help but I quickly went through the exchange on gcc-patches and got the impression that your work was already in an advanced stage, thus we should try to finish it! I'm pretty sure that we can make it in time for 4.8.0 (maybe with one or two more rounds of focused tips from Jason)
(In reply to Jonathan Wakely from comment #14) > Created attachment 20817 [details] > better -Wmeminit patch > > This version ignores empty classes and checks for a nontrivial default ctor > instead of layout_pod_type. > > This patch doesn't enable the warning unless explicity requested. I realise > that this warning is about enforcing style ("members should be initialised > in the mem-initializer-list not in the ctor body") but that's ok because > it's my preferred style, I just don't want the compiler to enforce other > people's preferred style ;) Perhaps a better alternative is to warn only if the uninitialized member is used in a mem-initializer. Then, when building the constructor call, mark the uninitialized members somehow as uninitialized for the middle-end, and let the middle-end handle the cases in the body of the constructor. The first part would already fix PR19808. The second part will fix this bug with fewer false positives than the proposed patch.
(In reply to Manuel López-Ibáñez from comment #20) > Perhaps a better alternative is to warn only if the uninitialized member is > used in a mem-initializer. Then, when building the constructor call, mark > the uninitialized members somehow as uninitialized for the middle-end, and > let the middle-end handle the cases in the body of the constructor. The > first part would already fix PR19808. The second part will fix this bug with > fewer false positives than the proposed patch. And this comment gives some ideas: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=19808#c9 Richard B, you say that: "using <error-mark> [for marking undefined memory] is just a random (and probably bad) idea." Intuitively this seems more scalable, since one could propagate the undefined value like VRP does. The other alternative "Introducing a SSA name default definition for A (even though not of register type)", however, seems possible already. no? We do already check for default definitions in virtual operands, tree-ssa-uninit.c says: /* For memory the only cheap thing we can do is see if we have a use of the default def of the virtual operand. So what is missing here?
I have a fix for this, incremental over the PR79345 one. t.C: In constructor ‘A::A()’: t.C:7:11: warning: ‘*<unknown>.A::g’ is used uninitialized in this function [-Wuninitialized] f = g; ^ comes at the cost of initializing pointer-based refs as opposed to only var-based (so this cost can be non-trivial).
*** Bug 19808 has been marked as a duplicate of this bug. ***
Author: rguenth Date: Mon Apr 24 07:34:51 2017 New Revision: 247090 URL: https://gcc.gnu.org/viewcvs?rev=247090&root=gcc&view=rev Log: 2017-04-24 Richard Biener <rguenther@suse.de> PR c++/2972 * tree-ssa-uninit.c (warn_uninitialized_vars): Handle some pointer-based references. * g++.dg/warn/Wuninitialized-10.C: New testcase. Added: trunk/gcc/testsuite/g++.dg/warn/Wuninitialized-10.C Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-uninit.c
Fixed for GCC 8.