On Tue, 6 Dec 2022, Patrick Palka wrote:

> This patch fixes some issues with substitution into a C++20 template
> parameter object wrapper:
> 
> * The first testcase demonstrates a situation where the same_type_p
>   assert in relevant case of tsubst_copy doesn't hold, because (partial)
>   substitution of {int,} into the VIEW_CONVERT_EXPR wrapper yields
>   A<int> but substitution into the underlying TEMPLATE_PARM_INDEX is a
>   nop and yields A<T> due to tsubst's level == 1 early exit test.  So
>   this patch just gets rid of the assert; the type mismatch doesn't
>   seem to be a problem in practice since the coercion is from one
>   dependent type to another.
> 
> * In the second testcase, dependent substitution into the underlying
>   TEMPLATE_PARM_INDEX yields a CALL_EXPR with empty TREE_TYPE, which
>   tsubst_copy doesn't expect.  This patch fixes this by handling empty
>   TREE_TYPE the same way as a non-const TREE_TYPE.  Moreover, after
>   this substitution we're left with a VIEW_CONVERT_EXPR wrapping a
>   CALL_EXPR instead of a TEMPLATE_PARM_INDEX, which during the subsequent
>   non-dependent substitution tsubst_copy doesn't expect either.  So
>   this patch also relaxes the tsubst_copy case to accept such
>   VIEW_CONVERT_EXPR too.
> 
> * In the third testcase, we end up never resolving the call to
>   f.modify() since tsubst_copy doesn't do overload resolution.
>   This patch fixes this by moving the handling of these
>   VIEW_CONVERT_EXPR wrappers from tsubst_copy to tsubst_copy_and_build.
>   And it turns out (at least according to our testsuite) that
>   tsubst_copy doesn't directly need to handle the other kinds of
>   NON_LVALUE_EXPR and VIEW_CONVERT_EXPR, so this patch also gets rid
>   of the location_wrapper_p handling from tsubst_copy and moves the
>   REF_PARENTHESIZED_P handling to tsubst_copy_and_build.

On second thought, getting rid of the location_wrapper_p and
REF_PARENTHESIZED_P handling from tsubst_copy is perhaps too
risky at this stage.  The following patch instead just moves
the tparm object wrapper handling from tsubst_copy to
tsubst_copy_and_build and leaves the rest of tsubst_copy alone.
Smoke tested so far, full bootstrap+regtest in progress:

-- >8--

Subject: [PATCH] c++: NTTP object wrapper substitution fixes [PR103346, ...]

This patch fixes some issues with substitution into a C++20 template
parameter object wrapper:

* The first testcase demonstrates a situation where the same_type_p
  assert in relevant case of tsubst_copy doesn't hold, because (partial)
  substitution of {int,} into the VIEW_CONVERT_EXPR wrapper yields
  A<int> but substitution into the underlying TEMPLATE_PARM_INDEX is a
  nop and yields A<T> due to tsubst's level == 1 early exit test.  So
  this patch just gets rid of the assert; the type mismatch doesn't
  seem to be a problem in practice, I suppose because the coercion is
  from one dependent type to another.

* In the second testcase, dependent substitution into the underlying
  TEMPLATE_PARM_INDEX yields a CALL_EXPR with empty TREE_TYPE, which
  tsubst_copy doesn't expect.  This patch fixes this by handling empty
  TREE_TYPE the same way as a non-const TREE_TYPE.  Moreover, after
  this substitution we're left with a VIEW_CONVERT_EXPR wrapping a
  CALL_EXPR instead of a TEMPLATE_PARM_INDEX, which during the subsequent
  non-dependent substitution tsubst_copy doesn't expect either.  So
  this patch also relaxes tsubst_copy to accept such VIEW_CONVERT_EXPR
  too.

* In the third testcase, we end up never resolving the call to
  f.modify() since tsubst_copy doesn't do overload resolution.
  This patch fixes this by moving the handling of these
  VIEW_CONVERT_EXPR wrappers from tsubst_copy to tsubst_copy_and_build.
  For good measure tsubst_copy_and_build should also handle
  REF_PARENTHESIZED_P wrappers instead of delegating to tsubst_copy.

After this patch, VIEW_CONVERT_EXPR substitution is ultimately just
moved from tsubst_copy to tsubst_copy_and_build and made more
permissive.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

        PR c++/103346
        PR c++/104278
        PR c++/102553

gcc/cp/ChangeLog:

        * pt.cc (tsubst_copy) <case VIEW_CONVERT_EXPR>: In the handling
        of C++20 template parameter object wrappers: Remove same_type_p
        assert.  Accept non-TEMPLATE_PARM_INDEX inner operand.  Handle
        empty TREE_TYPE on substituted inner operand.  Move it to ...
        (tsubst_copy_and_build): ... here.  Also handle REF_PARENTHESIZED_P
        VIEW_CONVERT_EXPRs.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp2a/nontype-class52a.C: New test.
        * g++.dg/cpp2a/nontype-class53.C: New test.
        * g++.dg/cpp2a/nontype-class54.C: New test.
        * g++.dg/cpp2a/nontype-class55.C: New test.
---
 gcc/cp/pt.cc                                  | 73 ++++++++++---------
 gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C | 15 ++++
 gcc/testsuite/g++.dg/cpp2a/nontype-class53.C  | 25 +++++++
 gcc/testsuite/g++.dg/cpp2a/nontype-class54.C  | 23 ++++++
 gcc/testsuite/g++.dg/cpp2a/nontype-class55.C  | 15 ++++
 5 files changed, 116 insertions(+), 35 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class53.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class54.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class55.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2d8e4fdd4b5..0a196f069ad 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -17271,42 +17271,16 @@ tsubst_copy (tree t, tree args, tsubst_flags_t 
complain, tree in_decl)
              return maybe_wrap_with_location (op0, EXPR_LOCATION (t));
            }
          tree op = TREE_OPERAND (t, 0);
-         if (code == VIEW_CONVERT_EXPR
-             && TREE_CODE (op) == TEMPLATE_PARM_INDEX)
-           {
-             /* Wrapper to make a C++20 template parameter object const.  */
-             op = tsubst_copy (op, args, complain, in_decl);
-             if (!CP_TYPE_CONST_P (TREE_TYPE (op)))
-               {
-                 /* The template argument is not const, presumably because
-                    it is still dependent, and so not the const template parm
-                    object.  */
-                 tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
-                 gcc_checking_assert (same_type_ignoring_top_level_qualifiers_p
-                                      (type, TREE_TYPE (op)));
-                 if (TREE_CODE (op) == CONSTRUCTOR
-                     || TREE_CODE (op) == IMPLICIT_CONV_EXPR)
-                   {
-                     /* Don't add a wrapper to these.  */
-                     op = copy_node (op);
-                     TREE_TYPE (op) = type;
-                   }
-                 else
-                   /* Do add a wrapper otherwise (in particular, if op is
-                      another TEMPLATE_PARM_INDEX).  */
-                   op = build1 (code, type, op);
-               }
-             return op;
-           }
          /* force_paren_expr can also create a VIEW_CONVERT_EXPR.  */
-         else if (code == VIEW_CONVERT_EXPR && REF_PARENTHESIZED_P (t))
+         if (code == VIEW_CONVERT_EXPR && REF_PARENTHESIZED_P (t))
            {
              op = tsubst_copy (op, args, complain, in_decl);
              op = build1 (code, TREE_TYPE (op), op);
              REF_PARENTHESIZED_P (op) = true;
              return op;
            }
-         /* We shouldn't see any other uses of these in templates.  */
+         /* We shouldn't see any other uses of these in templates
+            (tsubst_copy_and_build handles C++20 tparm object wrappers).  */
          gcc_unreachable ();
        }
 
@@ -21569,12 +21543,41 @@ tsubst_copy_and_build (tree t,
 
     case NON_LVALUE_EXPR:
     case VIEW_CONVERT_EXPR:
-      if (location_wrapper_p (t))
-       /* We need to do this here as well as in tsubst_copy so we get the
-          other tsubst_copy_and_build semantics for a PARM_DECL operand.  */
-       RETURN (maybe_wrap_with_location (RECUR (TREE_OPERAND (t, 0)),
-                                         EXPR_LOCATION (t)));
-      /* fallthrough.  */
+      {
+       tree op = RECUR (TREE_OPERAND (t, 0));
+       if (location_wrapper_p (t))
+         /* We need to do this here as well as in tsubst_copy so we get the
+            other tsubst_copy_and_build semantics for a PARM_DECL operand.  */
+         RETURN (maybe_wrap_with_location (op, EXPR_LOCATION (t)));
+
+       gcc_checking_assert (TREE_CODE (t) == VIEW_CONVERT_EXPR);
+       if (REF_PARENTHESIZED_P (t))
+         /* force_paren_expr can also create a VIEW_CONVERT_EXPR.  */
+         RETURN (finish_parenthesized_expr (op));
+
+       /* Otherwise, we're dealing with a wrapper to make a C++20 template
+          parameter object const.  */
+       if (TREE_TYPE (op) == NULL_TREE
+           || !CP_TYPE_CONST_P (TREE_TYPE (op)))
+         {
+           /* The template argument is not const, presumably because
+              it is still dependent, and so not the const template parm
+              object.  */
+           tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
+           if (TREE_CODE (op) == CONSTRUCTOR
+               || TREE_CODE (op) == IMPLICIT_CONV_EXPR)
+             {
+               /* Don't add a wrapper to these.  */
+               op = copy_node (op);
+               TREE_TYPE (op) = type;
+             }
+           else
+             /* Do add a wrapper otherwise (in particular, if op is
+                another TEMPLATE_PARM_INDEX).  */
+             op = build1 (VIEW_CONVERT_EXPR, type, op);
+         }
+       RETURN (op);
+      }
 
     default:
       /* Handle Objective-C++ constructs, if appropriate.  */
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C 
b/gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C
new file mode 100644
index 00000000000..ae5d5df70ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class52a.C
@@ -0,0 +1,15 @@
+// A version of nontype-class52.C where explicit template arguments are
+// given in the call to f (which during deduction need to be partially
+// substituted into the NTTP object V in f's signature).
+// { dg-do compile { target c++20 } }
+
+template<class> struct A { };
+
+template<auto> struct B { };
+
+template<class T, A<T> V> void f(B<V>);
+
+int main() {
+  constexpr A<int> a;
+  f<int>(B<a>{});
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class53.C 
b/gcc/testsuite/g++.dg/cpp2a/nontype-class53.C
new file mode 100644
index 00000000000..9a6398c5f57
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class53.C
@@ -0,0 +1,25 @@
+// PR c++/103346
+// { dg-do compile { target c++20 } }
+
+struct Item {};
+
+template<class T, T... ts>
+struct Sequence { };
+
+template<Item... items>
+using ItemSequence = Sequence<Item, items...>;
+
+template<Item... items>
+constexpr auto f() {
+  constexpr auto l = [](Item item) { return item; };
+  return ItemSequence<l(items)...>{};
+}
+
+using ty0 = decltype(f<>());
+using ty0 = ItemSequence<>;
+
+using ty1 = decltype(f<{}>());
+using ty1 = ItemSequence<{}>;
+
+using ty3 = decltype(f<{}, {}, {}>());
+using ty3 = ItemSequence<{}, {}, {}>;
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class54.C 
b/gcc/testsuite/g++.dg/cpp2a/nontype-class54.C
new file mode 100644
index 00000000000..8127b1f5426
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class54.C
@@ -0,0 +1,23 @@
+// PR c++/104278
+// { dg-do compile { target c++20 } }
+
+struct foo {
+  int value;
+  constexpr foo modify() const { return { value + 1 }; }
+};
+
+template<foo f, bool Enable = f.value & 1>
+struct bar {
+  static void run() { }
+};
+
+template<foo f>
+struct qux {
+  static void run() {
+    bar<f.modify()>::run();
+  }
+};
+
+int main() {
+  qux<foo{}>::run();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class55.C 
b/gcc/testsuite/g++.dg/cpp2a/nontype-class55.C
new file mode 100644
index 00000000000..afcb3d64495
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class55.C
@@ -0,0 +1,15 @@
+// PR c++/102553
+// { dg-do compile { target c++20 } }
+
+struct s1{};
+template<int> inline constexpr s1 ch{};
+
+template<s1 f> struct s2{};
+template<s1 f> using alias1 = s2<f>;
+
+template<class T>
+void general(int n) {
+  alias1<ch<1>>{};
+}
+
+template void general<int>(int);
-- 
2.39.0.rc2

Reply via email to