On Sun, Nov 07, 2021 at 04:48:46PM -0700, Martin Sebor wrote:
> On 11/5/21 5:22 PM, Marek Polacek via Gcc-patches wrote:
> > 2021 update: Last year I posted a version of this patch:
> > <https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559162.html>
> > but it didn't make it in. The main objection seemed to be that the
> > patch tried to do too much, and overlapped with the ME uninitialized
> > warnings. Since the patch used walk_tree without any data flow info,
> > it issued false positives for things like a(0 ? b : 42) and similar.
> >
> > I'll admit I've been dreading resurrecting this because of the lack
> > of clarity about where we should warn about what. On the other hand,
> > I think we really should do something about this. So I've simplified
> > the original patch as much as it seemed reasonable. For instance, it
> > doesn't even attempt to handle cases like "a((b = 42)), c(b)" -- for
> > these I simply give up for the whole mem-initializer (but who writes
> > code like that, anyway?). I also give up when a member is initialized
> > with a function call, because we don't know what the call could do.
>
> I (still) believe this is a great improvement :)
Yea, would be nice to finally nail it down this time around.
> Thinking about your last sentence above and experimenting with
> the patch just a little, we do know that function calls cannot
> "initialize" (by assigning a value) const or reference members,
> so those can be assumed to be uninitiazed when used before
> their initialized has been evaluated. As in:
>
> int f (int);
>
> struct A
> {
> int x;
> const int y;
> A ():
> x (f (y)), << y used before initialized
> y (1) { }
> };
>
> It should also be reasonable for the prurposes of warnings to
> assume that a call to a const member function (or any call where
> an object is passed by a const reference) doesn't change any
> non-mutable members. The middle end issues -Wmaybe-uninitialized
> when it sees an uninitualized object passed to a const-qualified
> pointer or reference.
Maybe, I don't know. It gets tricky and I don't think I want to
handle it in my patch, at least not in this initial version. It
may be best to leave this to the ME. Dunno.
> At the same time, a call to a non-const member function can assign
> a value to a not-yet-initialized member, so I wonder if its uses
> subsequent to it should be accepted without warning:
>
> struct A
> {
> int x, y, z;
> int f () { y = 1; return 2; }
> A ():
> x (f ()),
> z (y) << avoid warning here?
> { }
> };
This is actually a bug in my code, now fixed (made changes in the
DECL_NONSTATIC_MEMBER_FUNCTION_P block).
> > + /* We're initializing a reference member with itself. */
> > + if (TYPE_REF_P (type) && cp_tree_equal (d->member, init))
> > + warning_at (EXPR_LOCATION (init), OPT_Winit_self,
> > + "%qD is initialized with itself", field);
> > + else if (cp_tree_equal (TREE_OPERAND (init, 0), current_class_ref)
> > + && uninitialized->contains (field))
> > + {
> > + if (TYPE_REF_P (TREE_TYPE (field)))
> > + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized,
> > + "reference %qD is not yet bound to a value when used "
> > + "here", field);
> > + else if (!INDIRECT_TYPE_P (type) || is_this_parameter (d->member))
> > + warning_at (EXPR_LOCATION (init), OPT_Wuninitialized,
> > + "field %qD is used uninitialized", field);
>
> Might I suggest the word "member" instead?
>
> This is my OCD rearing its head but I always cringe when I read
> the word field in reference to a subobject. Field is a term
> common in database programming but in C and C++, either member
> or subobject are more wide-spread. Except for bit-fields, C
> formally uses the term field is to refer to the are into which
> printf directives format their output. With one exception, C++
> 98 used it the same way. I see that in recent versions another
> reference has crept in with the memory model, but I think it
> would be fair to call these two anomalies.
Hahah! No worries, changed to "member". "Field" is probably just
a GCCism in my head (due to FIELD_DECL).
> > +++ b/gcc/testsuite/g++.dg/warn/Wuninitialized-14.C
> > @@ -0,0 +1,31 @@
> > +// PR c++/19808
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-Wuninitialized" }
> > +
> > +struct A {
> > + int m;
> > + int get() const { return m; }
> > +
> > + A() { }
> > + A(int) { }
> > + A(const A &) { }
> > + A(A *) { }
> > +};
> > +
> > +struct S {
> > + A a, b;
> > +
> > + S(int (*)[1]) : a() {}
> > + S(int (*)[2]) : b(a.get()) {}
>
> Here, because S::a.m isn't initialized when a.get() returns it,
> we ideally want a warning, we just can't get it, right?
>
> To make this clear to people who read tests and also easier to
> add a warning if GCC gets smart enough (without causing what
> might look like false positives), I'd suggest to write the tests
> so that they will not trigger these "ideal" warnings. (In
> the above, it should be doable simply by initializing A::m in
> A's ctors.)
Right, we don't warn though it wouldn't be wrong to warn. I've
added ": m{}" to make 'm' initialized.
Regtest with the updated patch running...
Marek
