PR c/102245: Don't warn that ((_Bool)x<<0) isn't a truthvalue.

  If the tree expression X is a truthvalue, then X << 0 is a truthvalue.
In fact, because _Bool (truthvalue_type) has 1 bit precision, and shifts
are only well defined for bit counts less than the precision, the only
reasonable(?) left shift of a _Bool is by zero [where this reasonable
overlooks that shifts by zero should be optimized away as no-ops].

Now consider a language front-end that doesn't fold binary expressions,
hence retains (x<<0), but does fold type conversions, and can therefore
see that ((_Bool)x<<0) can be shortened to _Bool, but then warns that
any LSHIFT_EXPR in a boolean context is suspicious.

The answer is that shifts by zero are special, and that all other
shifts are indeed suspicious.  The most suspicious thing about a
(BImode) shift by zero, is why it hasn't already been optimized away.
Indeed, in Bernd Edlinger's original 2016 patch submission to warn
of LSHIFT_EXPR with -Wint-in-bool-context he included exceptions
for shifts (of truthvalues) by zero,
https://gcc.gnu.org/pipermail/gcc-patches/2016-September/457716.html
but was talked out of this during the review process, and unconditionally
warned of all LSHIFT_EXPRs by
https://gcc.gnu.org/pipermail/gcc-patches/2016-September/458263.html

This patch teaches c_common_truthvalue_conversion that a left shift
by zero is special/a no-op, and to apply the conversion to the first
operand, which both fixes the bogus warning and generates more sensible
expression trees.  [Some part of me thinks increasing the amount of
folding in the front-ends is bad, but another part thinks that calling
fold on trees that haven't had their operands folded/canonicalized
(then complaining about suspicious looking but perfectly valid results)
is sometimes worse].

This patch has been tested on x86_64-pc-linux-gnu with "make bootstrap"
and "make -k check" with no new failures.  Ok for mainline?


2021-09-13  Roger Sayle  <roger@nextmovesoftware.com>

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
--

/* PR c/102245 */
/* { dg-options "-Wint-in-bool-context" } */
/* { dg-do compile } */

_Bool test1(_Bool x)
{
  return !(x << 0);  /* { dg-bogus "boolean context" } */
}

_Bool test2(_Bool x)
{
  return !(x << 1);  /* { dg-warning "boolean context" } */
}

_Bool test3(_Bool x, int y)
{
  return !(x << y);  /* { dg-warning "boolean context" } */
}

_Bool test4(int x, int y)
{
  return !(x << y);  /* { dg-warning "boolean context" } */
}

_Bool test5(int x, int y)
{
  return !((x << y) << 0);  /* { dg-warning "boolean context" } */
}

int test6(_Bool x)
{
  int v = 0;
  return (v & ~1L) | (1L & (x << 0));  /* { dg-bogus "boolean context" } */
}
  

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