Re: [C PATCH] Don't leak C_MAYBE_CONST_EXPRs into fold() (PR c/68412)

[Marek Polacek <polacek at redhat dot com>]

On Wed, Nov 18, 2015 at 05:03:48PM +0100, Marek Polacek wrote:
> Since C++ delayed folding, warn_tautological_cmp needs to fold its arguments.
> But sometimes this function gets C_MAYBE_CONST_EXPR from the C FE, and fold()
> duly ICEs.
> 
> I was torn if I should just return from warn_tautological_cmp and not warn
> when it gets C_MAYBE_CONST_EXPR as an argument, or if I should strip
> C_MAYBE_CONST_EXPR before calling warn_tautological_cmp.  I decided for the
> latter since I think warn_tautological_cmp probably should warn on code such
> as
>   int i = 10;
>   bool b = ({ i; }) == ({ i; });
> (ugh).
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

Actually, no, I think we should do this instead.

Ok if testing passes?

2015-11-18  Marek Polacek  <polacek@redhat.com>

	PR c/68412
	* c-common.c (warn_tautological_cmp): Don't fold arguments here.

	* call.c (build_new_op_1): Fold arguments to warn_tautological_cmp.

	* gcc.dg/pr68412.c: New test.

diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index f50ca48..06d857c 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1924,7 +1924,7 @@ warn_tautological_cmp (location_t loc, enum tree_code code, tree lhs, tree rhs)
 
   /* We do not warn for constants because they are typical of macro
      expansions that test for features, sizeof, and similar.  */
-  if (CONSTANT_CLASS_P (fold (lhs)) || CONSTANT_CLASS_P (fold (rhs)))
+  if (CONSTANT_CLASS_P (lhs) || CONSTANT_CLASS_P (rhs))
     return;
 
   /* Don't warn for e.g.
diff --git gcc/cp/call.c gcc/cp/call.c
index 8cdda62..77c2936 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -5741,7 +5741,7 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1,
 	maybe_warn_bool_compare (loc, code, fold (arg1),
 				 fold (arg2));
       if (complain & tf_warning && warn_tautological_compare)
-	warn_tautological_cmp (loc, code, arg1, arg2);
+	warn_tautological_cmp (loc, code, fold (arg1), fold (arg2));
       /* Fall through.  */
     case PLUS_EXPR:
     case MINUS_EXPR:
diff --git gcc/testsuite/gcc.dg/pr68412.c gcc/testsuite/gcc.dg/pr68412.c
index e69de29..825eb63 100644
--- gcc/testsuite/gcc.dg/pr68412.c
+++ gcc/testsuite/gcc.dg/pr68412.c
@@ -0,0 +1,41 @@
+/* PR c/68412 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+#define M (sizeof (int) * __CHAR_BIT__)
+
+int
+fn1 (int i)
+{
+  return i == (-1 << 8); /* { dg-warning "left shift of negative value" } */
+}
+
+int
+fn2 (int i)
+{
+  return i == (1 << M); /* { dg-warning "left shift count" } */
+}
+
+int
+fn3 (int i)
+{
+  return i == 10 << (M - 1); /* { dg-warning "requires" } */
+}
+
+int
+fn4 (int i)
+{
+  return i == 1 << -1; /* { dg-warning "left shift count" } */
+}
+
+int
+fn5 (int i)
+{
+  return i == 1 >> M; /* { dg-warning "right shift count" } */
+}
+
+int
+fn6 (int i)
+{
+  return i == 1 >> -1; /* { dg-warning "right shift count" } */
+}

	Marek