diff mbox

[C++,Patch/RFC] PR 67980 ("left shift count is negative [-Wshift-count-negative] generated for unreachable code")

Message ID be514d86-3ff3-c231-ae7f-0af8d9646384@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 18, 2016, 4:17 p.m. UTC 
Hi,

in the language of our implementations details, submitter noticed that 
in terms of warnings we handle in a different way COND_EXPRs in 
tsubst_copy_and_build - we use fold_non_dependent_expr and integer_zerop 
to suppress undesired warnings by bumping c_inhibit_evaluation_warnings 
- and IF_STMTs in tsubst_expr, where we don't. My patch below, which 
passes testing, tries in a rather straightforward way to adopt the same 
mechanisms in the latter. There are quite a few details I'm not sure 
about: whether we should only use fold_non_dependent_expr for the 
purpose of suppressing the warnings -  thus never touching 'tmp' in the 
pt.c code handling IF_STMTs - which would be completely conservative in 
terms of code generation; whether there are subtle interactions with the 
new if constexpr, which I'm missing at the moment.

Thanks!

Paolo.

//////////////////
diff mbox

Patch

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 241297)
+++ cp/pt.c	(working copy)
@@ -15403,26 +15403,46 @@ 
       break;
 
     case IF_STMT:
-      stmt = begin_if_stmt ();
-      IF_STMT_CONSTEXPR_P (stmt) = IF_STMT_CONSTEXPR_P (t);
-      tmp = RECUR (IF_COND (t));
-      tmp = finish_if_stmt_cond (tmp, stmt);
-      if (IF_STMT_CONSTEXPR_P (t) && integer_zerop (tmp))
-	/* Don't instantiate the THEN_CLAUSE. */;
-      else
-	RECUR (THEN_CLAUSE (t));
-      finish_then_clause (stmt);
+      {
+	tree folded_tmp;
+	bool zerop, nonzerop;
 
-      if (IF_STMT_CONSTEXPR_P (t) && integer_nonzerop (tmp))
-	/* Don't instantiate the ELSE_CLAUSE. */;
-      else if (ELSE_CLAUSE (t))
-	{
-	  begin_else_clause (stmt);
-	  RECUR (ELSE_CLAUSE (t));
-	  finish_else_clause (stmt);
-	}
+	stmt = begin_if_stmt ();
+	IF_STMT_CONSTEXPR_P (stmt) = IF_STMT_CONSTEXPR_P (t);
+	tmp = RECUR (IF_COND (t));
+	folded_tmp = fold_non_dependent_expr (tmp);
+	if (TREE_CODE (folded_tmp) == INTEGER_CST)
+	  tmp = folded_tmp;
+	tmp = finish_if_stmt_cond (tmp, stmt);
+	zerop = integer_zerop (tmp);
+	nonzerop = integer_nonzerop (tmp);
+	if (IF_STMT_CONSTEXPR_P (t) && zerop)
+	  /* Don't instantiate the THEN_CLAUSE. */;
+	else
+	  {
+	    if (zerop)
+	      ++c_inhibit_evaluation_warnings;
+	    RECUR (THEN_CLAUSE (t));
+	    if (zerop)
+	      --c_inhibit_evaluation_warnings;
+	  }
+	finish_then_clause (stmt);
 
-      finish_if_stmt (stmt);
+	if (IF_STMT_CONSTEXPR_P (t) && nonzerop)
+	  /* Don't instantiate the ELSE_CLAUSE. */;
+	else if (ELSE_CLAUSE (t))
+	  {
+	    begin_else_clause (stmt);
+	    if (nonzerop)
+	      ++c_inhibit_evaluation_warnings;
+	    RECUR (ELSE_CLAUSE (t));
+	    if (nonzerop)
+	      --c_inhibit_evaluation_warnings;
+	    finish_else_clause (stmt);
+	  }
+
+	finish_if_stmt (stmt);
+      }
       break;
 
     case BIND_EXPR:
Index: testsuite/g++.dg/cpp1y/pr67980.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr67980.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/pr67980.C	(working copy)
@@ -0,0 +1,23 @@ 
+// { dg-do compile { target c++14 } }
+
+template <int Y, class T>
+constexpr T cpp14_constexpr_then(T value) {
+  if (Y < 0)
+    return (value << -Y);
+  else
+    return 0;
+}
+
+template <int Y, class T>
+constexpr T cpp14_constexpr_else(T value) {
+  if (Y > 0)
+    return 0;
+  else
+    return (value << -Y);
+}
+
+int main()
+{
+  cpp14_constexpr_then<1>(0);
+  cpp14_constexpr_else<1>(0);
+}