[PATCH] c++: extend lookup_template_class shortcut [PR110122]

[PATCH] c++: extend lookup_template_class shortcut [PR110122]

[Patrick Palka]

Here when substituting the injected class name A during regeneration of
the lambda, we find ourselves in lookup_template_class for A with
V=_ZTAXtl3BarEE (i.e. the template parameter object for Foo{}).  The
call to coerce_template_parms within then undesirably tries to make a copy
of this class NTTP argument, which fails because its type is not copyable.

Sidestepping the question of when precisely a class NTTP should be
copied (which is the subject of PR104577), it seems clear that this
testcase shouldn't require copyability of Foo.

lookup_template_class has a shortcut for looking up the current class
scope, which would avoid the problematic coerce_template_parms call, but
the shortcut doesn't trigger because it only considers the innermost
class scope which in this case in the lambda type.  So this patch fixes
this by extending the lookup_template_class shortcut to consider outer
class scopes too (and skipping over lambda types since they are never
instantiated from lookup_template_class IIUC).  We also need to avoid
calling coerce_template_parms when looking up a templated non-template
class for sake of the A::B example.  The call should be unnecessary
because the innermost arguments belong to the context and so should have
already been coerced.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?  To fix the 13 regression in that PR, it should suffice to do a
minimal backport of this change that just replaces current_class_type
with current_nonlambda_class_type in the shortcut.

	PR c++/110122
	PR c++/104577

gcc/cp/ChangeLog:

	* pt.cc (lookup_template_class): Extend shortcut for looking
	up the current class scope to consider outer class scopes too,
	and use current_nonlambda_class_type instead of current_class_type.
	Only call coerce_template_parms when looking up a true class
	template.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/nontype-class57.C: New test.
---
 gcc/cp/pt.cc                                 | 19 +++++++++++----
 gcc/testsuite/g++.dg/cpp2a/nontype-class57.C | 25 ++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class57.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 244b0b03454..29ad9ba4072 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -9928,16 +9928,27 @@ lookup_template_class (tree d1, tree arglist, tree in_decl, tree context,
 	 template.  */
 
       /* Shortcut looking up the current class scope again.  */
-      if (current_class_type)
-	if (tree ti = CLASSTYPE_TEMPLATE_INFO (current_class_type))
+      for (tree scope = current_nonlambda_class_type ();
+	   scope != NULL_TREE;
+	   scope = TYPE_P (scope) ? TYPE_CONTEXT (scope) : DECL_CONTEXT (scope))
+	{
+	  if (!CLASS_TYPE_P (scope))
+	    continue;
+
+	  tree ti = CLASSTYPE_TEMPLATE_INFO (scope);
+	  if (!ti || TMPL_ARGS_DEPTH (TI_ARGS (ti)) < arg_depth)
+	    break;
+
 	  if (gen_tmpl == most_general_template (TI_TEMPLATE (ti))
 	      && comp_template_args (arglist, TI_ARGS (ti)))
-	    return current_class_type;
+	    return scope;
+	}
 
       /* Calculate the BOUND_ARGS.  These will be the args that are
 	 actually tsubst'd into the definition to create the
 	 instantiation.  */
-      arglist = coerce_template_parms (parmlist, arglist, gen_tmpl, complain);
+      if (PRIMARY_TEMPLATE_P (gen_tmpl))
+	arglist = coerce_template_parms (parmlist, arglist, gen_tmpl, complain);
 
       if (arglist == error_mark_node)
 	/* We were unable to bind the arguments.  */
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class57.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class57.C
new file mode 100644
index 00000000000..88ebdc1b3ff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class57.C
@@ -0,0 +1,25 @@
+// PR c++/110122
+// { dg-do compile { target c++20 } }
+
+struct Foo {
+  constexpr Foo() = default;
+  Foo(const Foo&) = delete;
+};
+
+template<Foo V>
+struct A {
+  A() {
+    [] { A a; }();
+    [this] { this; }();
+  }
+
+  struct B {
+    B() {
+      [] { A a; }();
+      [this] { this; }();
+    }
+  };
+};
+
+A<Foo{}> a;
+A<Foo{}>::B b;
-- 
2.41.0.rc1.10.g9e49351c30

Re: [PATCH] c++: extend lookup_template_class shortcut [PR110122]

[Jason Merrill]

On 6/5/23 17:26, Patrick Palka wrote:
> Here when substituting the injected class name A during regeneration of
> the lambda, we find ourselves in lookup_template_class for A with
> V=_ZTAXtl3BarEE (i.e. the template parameter object for Foo{}).  The
> call to coerce_template_parms within then undesirably tries to make a copy
> of this class NTTP argument, which fails because its type is not copyable.
> 
> Sidestepping the question of when precisely a class NTTP should be
> copied (which is the subject of PR104577), it seems clear that this
> testcase shouldn't require copyability of Foo.
> 
> lookup_template_class has a shortcut for looking up the current class
> scope, which would avoid the problematic coerce_template_parms call, but
> the shortcut doesn't trigger because it only considers the innermost
> class scope which in this case in the lambda type.  So this patch fixes
> this by extending the lookup_template_class shortcut to consider outer
> class scopes too (and skipping over lambda types since they are never
> instantiated from lookup_template_class IIUC).  We also need to avoid
> calling coerce_template_parms when looking up a templated non-template
> class for sake of the A::B example.  The call should be unnecessary
> because the innermost arguments belong to the context and so should have
> already been coerced.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?  To fix the 13 regression in that PR, it should suffice to do a
> minimal backport of this change that just replaces current_class_type
> with current_nonlambda_class_type in the shortcut.

OK.

> 	PR c++/110122
> 	PR c++/104577
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (lookup_template_class): Extend shortcut for looking
> 	up the current class scope to consider outer class scopes too,
> 	and use current_nonlambda_class_type instead of current_class_type.
> 	Only call coerce_template_parms when looking up a true class
> 	template.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/nontype-class57.C: New test.
> ---
>   gcc/cp/pt.cc                                 | 19 +++++++++++----
>   gcc/testsuite/g++.dg/cpp2a/nontype-class57.C | 25 ++++++++++++++++++++
>   2 files changed, 40 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class57.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 244b0b03454..29ad9ba4072 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -9928,16 +9928,27 @@ lookup_template_class (tree d1, tree arglist, tree in_decl, tree context,
>   	 template.  */
>   
>         /* Shortcut looking up the current class scope again.  */
> -      if (current_class_type)
> -	if (tree ti = CLASSTYPE_TEMPLATE_INFO (current_class_type))
> +      for (tree scope = current_nonlambda_class_type ();
> +	   scope != NULL_TREE;
> +	   scope = TYPE_P (scope) ? TYPE_CONTEXT (scope) : DECL_CONTEXT (scope))
> +	{
> +	  if (!CLASS_TYPE_P (scope))
> +	    continue;
> +
> +	  tree ti = CLASSTYPE_TEMPLATE_INFO (scope);
> +	  if (!ti || TMPL_ARGS_DEPTH (TI_ARGS (ti)) < arg_depth)
> +	    break;
> +
>   	  if (gen_tmpl == most_general_template (TI_TEMPLATE (ti))
>   	      && comp_template_args (arglist, TI_ARGS (ti)))
> -	    return current_class_type;
> +	    return scope;
> +	}
>   
>         /* Calculate the BOUND_ARGS.  These will be the args that are
>   	 actually tsubst'd into the definition to create the
>   	 instantiation.  */
> -      arglist = coerce_template_parms (parmlist, arglist, gen_tmpl, complain);
> +      if (PRIMARY_TEMPLATE_P (gen_tmpl))
> +	arglist = coerce_template_parms (parmlist, arglist, gen_tmpl, complain);
>   
>         if (arglist == error_mark_node)
>   	/* We were unable to bind the arguments.  */
> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class57.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class57.C
> new file mode 100644
> index 00000000000..88ebdc1b3ff
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class57.C
> @@ -0,0 +1,25 @@
> +// PR c++/110122
> +// { dg-do compile { target c++20 } }
> +
> +struct Foo {
> +  constexpr Foo() = default;
> +  Foo(const Foo&) = delete;
> +};
> +
> +template<Foo V>
> +struct A {
> +  A() {
> +    [] { A a; }();
> +    [this] { this; }();
> +  }
> +
> +  struct B {
> +    B() {
> +      [] { A a; }();
> +      [this] { this; }();
> +    }
> +  };
> +};
> +
> +A<Foo{}> a;
> +A<Foo{}>::B b;