[gcc r13-1219] c++: -Waddress and if constexpr [PR94554]

[gcc r13-1219] c++: -Waddress and if constexpr [PR94554]

[Jason Merrill]

https://gcc.gnu.org/g:124a9e08b7a83795bd4d09001955f0eef68ecd00

commit r13-1219-g124a9e08b7a83795bd4d09001955f0eef68ecd00
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Jun 23 00:24:22 2022 -0400

    c++: -Waddress and if constexpr [PR94554]
    
    Like we avoid various warnings for seemingly tautological expressions when
    substituting a template, we should avoid warning for the implicit conversion
    to bool in an if statement.  I considered also doing this for the conditions
    in loop expressions, but that seems unnecessary, as a loop condition is
    unlikely to be a constant.
    
    The change to finish_if_stmt_cond isn't necessary since dependent_operand_p
    looks through IMPLICIT_CONV_EXPR, but makes it more constent with
    e.g. build_x_binary_op that determines the type of an expression and then
    builds it using the original operands.
    
            PR c++/94554
    
    gcc/cp/ChangeLog:
    
            * pt.cc (dependent_operand_p): Split out from...
            (tsubst_copy_and_build): ...here.
            (tsubst_expr) [IF_STMT]: Use it.
            * semantics.cc (finish_if_stmt_cond): Keep the pre-conversion
            condition in the template tree.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp1z/constexpr-if38.C: New test.

Diff:
---
 gcc/cp/pt.cc                                | 43 +++++++++++++++++++++--------
 gcc/cp/semantics.cc                         | 10 +++++--
 gcc/testsuite/g++.dg/cpp1z/constexpr-if38.C | 16 +++++++++++
 3 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 4535c14c26a..12a2b57d9e3 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -18547,6 +18547,29 @@ lookup_init_capture_pack (tree decl)
   return r;
 }
 
+/* T is an operand of a template tree being substituted.  Return whether
+   T is dependent such that we should suppress some warnings that would
+   make sense if the substituted expression were written directly, like
+     template <int I> bool f() { return I == 2; }
+   We don't want to warn when instantiating f that comparing two constants
+   always has the same value.
+
+   This is a more limited concept of dependence than instantiation-dependent;
+   here we don't care whether substitution could fail.  */
+
+static bool
+dependent_operand_p (tree t)
+{
+  while (TREE_CODE (t) == IMPLICIT_CONV_EXPR)
+    t = TREE_OPERAND (t, 0);
+  ++processing_template_decl;
+  bool r = (potential_constant_expression (t)
+	    ? value_dependent_expression_p (t)
+	    : type_dependent_expression_p (t));
+  --processing_template_decl;
+  return r;
+}
+
 /* Like tsubst_copy for expressions, etc. but also does semantic
    processing.  */
 
@@ -18872,8 +18895,13 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
       IF_STMT_CONSTEVAL_P (stmt) = IF_STMT_CONSTEVAL_P (t);
       if (IF_STMT_CONSTEXPR_P (t))
 	args = add_extra_args (IF_STMT_EXTRA_ARGS (t), args, complain, in_decl);
-      tmp = RECUR (IF_COND (t));
-      tmp = finish_if_stmt_cond (tmp, stmt);
+      {
+	tree cond = IF_COND (t);
+	bool was_dep = dependent_operand_p (cond);
+	cond = RECUR (cond);
+	warning_sentinel s1(warn_address, was_dep);
+	tmp = finish_if_stmt_cond (cond, stmt);
+      }
       if (IF_STMT_CONSTEXPR_P (t)
 	  && instantiation_dependent_expression_p (tmp))
 	{
@@ -20422,15 +20450,8 @@ tsubst_copy_and_build (tree t,
 	   warnings that depend on the range of the types involved.  */
 	tree op0 = TREE_OPERAND (t, 0);
 	tree op1 = TREE_OPERAND (t, 1);
-	auto dep_p = [](tree t) {
-	  ++processing_template_decl;
-	  bool r = (potential_constant_expression (t)
-		    ? value_dependent_expression_p (t)
-		    : type_dependent_expression_p (t));
-	  --processing_template_decl;
-	  return r;
-	};
-	const bool was_dep = dep_p (op0) || dep_p (op1);
+	const bool was_dep = (dependent_operand_p (op0)
+			      || dependent_operand_p (op1));
 	op0 = RECUR (op0);
 	op1 = RECUR (op1);
 
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 24cca99e909..2344b5eea00 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -1029,9 +1029,9 @@ maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
    IF_STMT.  */
 
 tree
-finish_if_stmt_cond (tree cond, tree if_stmt)
+finish_if_stmt_cond (tree orig_cond, tree if_stmt)
 {
-  cond = maybe_convert_cond (cond);
+  tree cond = maybe_convert_cond (orig_cond);
   if (IF_STMT_CONSTEXPR_P (if_stmt)
       && !type_dependent_expression_p (cond)
       && require_constant_expression (cond)
@@ -1045,7 +1045,11 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
       cond = cxx_constant_value (cond, NULL_TREE);
     }
   else
-    maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false);
+    {
+      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false);
+      if (processing_template_decl)
+	cond = orig_cond;
+    }
   finish_cond (&IF_COND (if_stmt), cond);
   add_stmt (if_stmt);
   THEN_CLAUSE (if_stmt) = push_stmt_list ();
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-if38.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-if38.C
new file mode 100644
index 00000000000..3e4fd446dca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-if38.C
@@ -0,0 +1,16 @@
+// PR c++/94554
+// { dg-do compile { target c++17 } }
+// { dg-additional-options -Wall }
+
+int meow() { return 1; }
+void kitty(int);
+template <int (*F)()>
+void test() {
+    if constexpr (F) {
+        kitty(F());
+    } else {
+        kitty(2);
+    }
+}
+template void test<nullptr>();
+template void test<meow>();