Message ID | 20200527002541.546573-1-polacek@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: Fix bogus -Wparentheses warning [PR95344] | expand |
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? Jason
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
On 5/28/20 7:11 PM, Marek Polacek wrote: > 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? OK. > -- >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 >
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); + } break; 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" } + { + } +}