| Message ID | 20170321153819.GM3172@redhat.com |
|---|---|
| State | New |
| Headers | show |
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)); > + 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
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? > > + if (!TREE_SIDE_EFFECTS (r)) > > + x = r; > > + break; > > + Jakub
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)); + 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()); +}