On Thu, May 28, 2020 at 05:01:51PM -0400, Jason Merrill wrote: > On 5/26/20 8:25 PM, Marek Polacek wrote: > > Since r267272, which added location wrappers, cp_fold loses > > TREE_NO_WARNING on a MODIFY_EXPR that finish_parenthesized_expr set, and > > that results in a bogus -Wparentheses warning. > > > > I.e., previously we had "b = 1" but now we have "VIEW_CONVERT_EXPR<bool>(b) > > = 1" > > and cp_fold_maybe_rvalue folds away the location wrapper and so we do > > 2718 x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1); > > in cp_fold and the flag is lost. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10/9? > > > > PR c++/95344 > > * cp-gimplify.c (cp_fold) <case MODIFY_EXPR>: Set TREE_NO_WARNING. > > > > * c-c++-common/Wparentheses-2.c: New test. > > --- > > gcc/cp/cp-gimplify.c | 5 ++++- > > gcc/testsuite/c-c++-common/Wparentheses-2.c | 18 ++++++++++++++++++ > > 2 files changed, 22 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/c-c++-common/Wparentheses-2.c > > > > diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c > > index 53d715dcd89..8b505dd878c 100644 > > --- a/gcc/cp/cp-gimplify.c > > +++ b/gcc/cp/cp-gimplify.c > > @@ -2745,7 +2745,10 @@ cp_fold (tree x) > > x = org_x; > > } > > if (code == MODIFY_EXPR && TREE_CODE (x) == MODIFY_EXPR) > > - TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); > > + { > > + TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); > > + TREE_NO_WARNING (x) = TREE_NO_WARNING (org_x); > > + } > > I wonder if we want to copy these flags lower down for any EXPR_P (x) where > TREE_CODE (x) == code?
Sounds good; I don't think we want to lose those flags when folding in general, not just for MODIFY_EXPR. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Since r267272, which added location wrappers, cp_fold loses TREE_NO_WARNING on a MODIFY_EXPR that finish_parenthesized_expr set, and that results in a bogus -Wparentheses warning. I.e., previously we had "b = 1" but now we have "VIEW_CONVERT_EXPR<bool>(b) = 1" and cp_fold_maybe_rvalue folds away the location wrapper and so we do 2718 x = fold_build2_loc (loc, code, TREE_TYPE (x), op0, op1); in cp_fold and the flag is lost. PR c++/95344 * cp-gimplify.c (cp_fold) <case MODIFY_EXPR>: Don't set TREE_THIS_VOLATILE here. (cp_fold): Set it here along with TREE_NO_WARNING. * c-c++-common/Wparentheses-2.c: New test. --- gcc/cp/cp-gimplify.c | 8 ++++++-- gcc/testsuite/c-c++-common/Wparentheses-2.c | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wparentheses-2.c diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index 53d715dcd89..d6723e44ec4 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -2744,8 +2744,6 @@ cp_fold (tree x) else x = org_x; } - if (code == MODIFY_EXPR && TREE_CODE (x) == MODIFY_EXPR) - TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); break; @@ -2994,6 +2992,12 @@ cp_fold (tree x) return org_x; } + if (EXPR_P (x) && TREE_CODE (x) == code) + { + TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x); + TREE_NO_WARNING (x) = TREE_NO_WARNING (org_x); + } + if (!c.evaluation_restricted_p ()) { fold_cache->put (org_x, x); diff --git a/gcc/testsuite/c-c++-common/Wparentheses-2.c b/gcc/testsuite/c-c++-common/Wparentheses-2.c new file mode 100644 index 00000000000..1aa5d314ae7 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wparentheses-2.c @@ -0,0 +1,18 @@ +// PR c++/95344 - bogus -Wparentheses warning. +// { dg-do compile } +// { dg-options "-Wparentheses" } + +#ifndef __cplusplus +# define bool _Bool +# define true 1 +# define false 0 +#endif + +void +f (int i) +{ + bool b = false; + if (i == 99 ? (b = true) : false) // { dg-bogus "suggest parentheses" } + { + } +} base-commit: 3d8d5ddb539a5254c7ef83414377f4c74c7701d4 -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA