On Tue, Sep 14, 2021 at 9:44 AM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
> Respecting Jakub's suggestion that it may be better to warn-on-valid for
> "if (x << 0)" as the author might have intended "if (x < 0)" [which will
> also warn when x is _Bool], the simplest way to resolve this regression
> is to disable the recently added fold transformation for shifts by zero;
> these will be optimized later (elsewhere). Guarding against integer_zerop
> is the simplest of three alternatives; the second being to only apply
> this transformation to GIMPLE and not GENERIC, and the third (potentially)
> being to explicitly handle shifts by zero here, with an (if cond then else),
> optimizing the expression to a convert, but awkwardly duplicating the
> more general transformation earlier in match.pd's shift simplifications.
>
> This patch has been tested (against a recent snapshot without build issues)
> on x86_64-pc-linux-gnu with "make bootstrap" and "make -k check" with no
> new failures. Note that test1 in the new testcase is changed from
> dg-bogus to dg-warning compared with version #1. Ok for mainline?
OK.
Thanks,
Richard.
> 2021-09-14 Roger Sayle <ro...@nextmovesoftware.com>
>
> gcc/ChangeLog
> PR c/102245
> * match.pd (shift optimizations): Disable recent sign-changing
> optimization for shifts by zero, these will be folded later.
>
> gcc/testsuite/ChangeLog
> PR c/102245
> * gcc.dg/Wint-in-bool-context-4.c: New test case.
>
>
> Roger
>
> -----Original Message-----
> From: Jakub Jelinek <ja...@redhat.com>
> Sent: 13 September 2021 11:58
> To: Roger Sayle <ro...@nextmovesoftware.com>
> Cc: 'GCC Patches' <gcc-patches@gcc.gnu.org>
> Subject: Re: [PATCH] PR c/102245: Don't warn that ((_Bool)x<<0) isn't a
> truthvalue.
>
> On Mon, Sep 13, 2021 at 11:42:08AM +0100, Roger Sayle wrote:
> > gcc/c-family/ChangeLog
> > PR c/102245
> > * c-common.c (c_common_truthvalue_conversion) [LSHIFT_EXPR]:
> > Special case (optimize) shifts by zero.
> >
> > gcc/testsuite/ChangeLog
> > PR c/102245
> > * gcc.dg/Wint-in-bool-context-4.c: New test case.
> >
> > Roger
> > --
> >
>
> > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index
> > 017e415..44b5fcc 100644
> > --- a/gcc/c-family/c-common.c
> > +++ b/gcc/c-family/c-common.c
> > @@ -3541,6 +3541,10 @@ c_common_truthvalue_conversion (location_t location,
> > tree expr)
> > break;
> >
> > case LSHIFT_EXPR:
> > + /* Treat shifts by zero as a special case. */
> > + if (integer_zerop (TREE_OPERAND (expr, 1)))
> > + return c_common_truthvalue_conversion (location,
> > + TREE_OPERAND (expr, 0));
> > /* We will only warn on signed shifts here, because the majority of
> > false positive warnings happen in code where unsigned arithmetic
> > was used in anticipation of a possible overflow.
>
> > /* PR c/102245 */
> > /* { dg-options "-Wint-in-bool-context" } */
> > /* { dg-do compile } */
> >
> > _Bool test1(_Bool x)
> > {
> > return !(x << 0); /* { dg-bogus "boolean context" } */ }
>
> While this exact case is unlikely a misspelling of !(x < 0) as no _Bool is
> less than zero and hopefully we get a warning for !(x < 0), what about _Bool
> test1a(int x) {
> return !(x << 0);
> }
> ? I think there is a non-zero chance this was meant to be !(x < 0) and the
> current
> pr102245.c: In function ‘test1a’:
> pr102245.c:3:14: warning: ‘<<’ in boolean context, did you mean ‘<’?
> [-Wint-in-bool-context]
> 3 | return !(x << 0);
> | ~~~^~~~~
> warning seems to be useful.
>
> Jakub
>
