Re: C++ PATCH to fix bogus maybe-uninitialized warning (PR c++/80119)

[Marek Polacek <polacek at redhat dot com>]

On Tue, Mar 21, 2017 at 08:41:01PM +0100, Jakub Jelinek wrote:
> On Tue, Mar 21, 2017 at 03:27:02PM -0400, Jason Merrill wrote:
> > OK.
> > 
> > On Tue, Mar 21, 2017 at 11:38 AM, Marek Polacek <polacek@redhat.com> wrote:
> > > This patch fixes a bogus maybe-uninitialized warning reported in the PR.
> > > The issue is that we're not able to fold away useless CLEANUP_POINT_EXPRs,
> > > as e.g. in
> > >   if (<<cleanup_point 0>>)
> > >    // bogus warning
> > > Here, the cleanup_point was built as <<cleanup_point 0 && (i = 4) != 0>>,
> > > which cp_fold_r reduces to <<cleanup_point 0>>, but leaves it as that and
> > > passes it to the gimplifier.
> > >
> > > Jakub suggested handling this in cp_fold.  fold_build_cleanup_point_expr says
> > > that "if the expression does not have side effects then we don't have to wrap
> > > it with a cleanup point expression", so I think the following should be safe.
> > >
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > >
> > > 2017-03-21  Marek Polacek  <polacek@redhat.com>
> > >
> > >         PR c++/80119
> > >         * cp-gimplify.c (cp_fold): Strip CLEANUP_POINT_EXPR if the expression
> > >         doesn't have side effects.
> > >
> > >         * g++.dg/warn/Wuninitialized-9.C: New test.
> > >
> > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
> > > index ebb5da9..b4319ca 100644
> > > --- gcc/cp/cp-gimplify.c
> > > +++ gcc/cp/cp-gimplify.c
> > > @@ -2056,6 +2056,14 @@ cp_fold (tree x)
> > >    code = TREE_CODE (x);
> > >    switch (code)
> > >      {
> > > +    case CLEANUP_POINT_EXPR:
> > > +      /* Strip CLEANUP_POINT_EXPR if the expression doesn't have side
> > > +        effects.  */
> > > +      r = cp_fold (TREE_OPERAND (x, 0));
> 
> Can CLEANUP_POINT_EXPR be an lvalue?  If not, maybe cp_fold_rvalue instead?

I ran the testing with some lvalue_p checks added and it seems a CLEANUP_POINT_EXPR
is never an lvalue, so I guess we can call cp_fold_rvalue.  Jason, is this still
ok?

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2017-03-21  Marek Polacek  <polacek@redhat.com>

	PR c++/80119
	* cp-gimplify.c (cp_fold): Strip CLEANUP_POINT_EXPR if the expression
	doesn't have side effects.

	* g++.dg/warn/Wuninitialized-9.C: New test.

diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c
index ebb5da9..b4319ca 100644
--- gcc/cp/cp-gimplify.c
+++ gcc/cp/cp-gimplify.c
@@ -2056,6 +2056,14 @@ cp_fold (tree x)
   code = TREE_CODE (x);
   switch (code)
     {
+    case CLEANUP_POINT_EXPR:
+      /* Strip CLEANUP_POINT_EXPR if the expression doesn't have side
+	 effects.  */
+      r = cp_fold_rvalue (TREE_OPERAND (x, 0));
+      if (!TREE_SIDE_EFFECTS (r))
+	x = r;
+      break;
+
     case SIZEOF_EXPR:
       x = fold_sizeof_expr (x);
       break;
diff --git gcc/testsuite/g++.dg/warn/Wuninitialized-9.C gcc/testsuite/g++.dg/warn/Wuninitialized-9.C
index e69de29..3432b4f 100644
--- gcc/testsuite/g++.dg/warn/Wuninitialized-9.C
+++ gcc/testsuite/g++.dg/warn/Wuninitialized-9.C
@@ -0,0 +1,19 @@
+// PR c++/80119
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wuninitialized" }
+
+#include <type_traits>
+
+template <bool b>
+void failing_function(std::integral_constant<bool, b>)
+{
+   int i;
+   if (b && (i = 4)) {
+      ++i; // { dg-bogus "may be used uninitialized" }
+   }
+}
+
+int main (void)
+{
+   failing_function(std::false_type());
+}

	Marek