[PATCH] c++: context completion in lookup_template_class [PR105982]

[PATCH] c++: context completion in lookup_template_class [PR105982]

[Patrick Palka]

The below testcase demonstrates that completion of the substituted
context during lookup_template_class can end up registering the desired
specialization for us in more cases than r13-1045-gcb7fd1ea85feea
anticipated.  In particular, it can happen for a non-dependent
specialization of a nested class as well.

During overload resolution with A's guides, we substitute the deduced
argument T=int into the TYPENAME_TYPE B::C (*).  This substitution calls
lookup_template_class for A<T>::B with T=int, which completes A<int>
for the first time, which recursively registers the desired specialization
of B for us.  The parent call to lookup_template_class then tries
registering the same specialization, which leads to a crash.

This patch fixes this by making lookup_template_class check the
specializations table after completion of the context iff necessary,
i.e. when the call to complete_type actually has an effect.

(*): Note that the TYPE_CONTEXT of this TYPENAME_TYPE is just the
RECORD_TYPE B instead of TYPENAME_TYPE A<T>::B.  Is this expected?
I'd think maybe_dependent_member_ref would rewrite the reference to B
in terms of another TYPENAME_TYPE.  This is why the testcase needs to
use 'typename B::C' instead of just 'B' -- maybe_dependent_member_ref
rewrites the use of B in the latter case but not the former.

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

	PR c++/105982

gcc/cp/ChangeLog:

	* pt.cc (lookup_template_class): After completion of the
	substituted context, check the table again iff the type was
	previously incomplete and complete_type made it complete.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1z/class-deduction111.C: New test.
---
 gcc/cp/pt.cc                                  | 21 +++++++++++--------
 .../g++.dg/cpp1z/class-deduction111.C         | 10 +++++++++
 2 files changed, 22 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction111.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 28edc6ae988..eeae867a816 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -10089,16 +10089,19 @@ lookup_template_class (tree d1, tree arglist, tree in_decl, tree context,
 	    {
 	      context = tsubst_aggr_type (context, arglist,
 					  complain, in_decl, true);
-	      context = complete_type (context);
-	      if (is_dependent_type && arg_depth > 1)
+	      /* Try completing the enclosing context if it's not already so.  */
+	      if (context != error_mark_node
+		  && !COMPLETE_TYPE_P (context))
 		{
-		  /* If this is a dependent nested specialization such as
-		     A<T>::B<U> [with T=int, U=U], then completion of A<int>
-		     could have caused to register the desired specialization
-		     of B already, so check the table again (33959).  */
-		  entry = type_specializations->find_with_hash (&elt, hash);
-		  if (entry)
-		    return entry->spec;
+		  context = complete_type (context);
+		  if (COMPLETE_TYPE_P (context))
+		    {
+		      /* Completion could have caused us to register the desired
+			 specialization for us, so check the table again.  */
+		      entry = type_specializations->find_with_hash (&elt, hash);
+		      if (entry)
+			return entry->spec;
+		    }
 		}
 	    }
 	}
diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction111.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction111.C
new file mode 100644
index 00000000000..2406529ea5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction111.C
@@ -0,0 +1,10 @@
+// PR c++/105982
+// { dg-do compile { target c++17 } }
+
+template<class T>
+struct A {
+  struct B { struct C { }; };
+  A(T, typename B::C);
+};
+
+A a(0, {});
-- 
2.37.0.rc1

Re: [PATCH] c++: context completion in lookup_template_class [PR105982]

[Jason Merrill]

On 6/23/22 10:48, Patrick Palka wrote:
> The below testcase demonstrates that completion of the substituted
> context during lookup_template_class can end up registering the desired
> specialization for us in more cases than r13-1045-gcb7fd1ea85feea
> anticipated.  In particular, it can happen for a non-dependent
> specialization of a nested class as well.
> 
> During overload resolution with A's guides, we substitute the deduced
> argument T=int into the TYPENAME_TYPE B::C (*).  This substitution calls
> lookup_template_class for A<T>::B with T=int, which completes A<int>
> for the first time, which recursively registers the desired specialization
> of B for us.  The parent call to lookup_template_class then tries
> registering the same specialization, which leads to a crash.
> 
> This patch fixes this by making lookup_template_class check the
> specializations table after completion of the context iff necessary,
> i.e. when the call to complete_type actually has an effect.
> 
> (*): Note that the TYPE_CONTEXT of this TYPENAME_TYPE is just the
> RECORD_TYPE B instead of TYPENAME_TYPE A<T>::B.  Is this expected?
> I'd think maybe_dependent_member_ref would rewrite the reference to B
> in terms of another TYPENAME_TYPE.  This is why the testcase needs to
> use 'typename B::C' instead of just 'B' -- maybe_dependent_member_ref
> rewrites the use of B in the latter case but not the former.

Hmm, perhaps maybe_dependent_member_ref needs to consider rewriting ctx 
as well.

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

OK.

> 	PR c++/105982
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (lookup_template_class): After completion of the
> 	substituted context, check the table again iff the type was
> 	previously incomplete and complete_type made it complete.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/class-deduction111.C: New test.
> ---
>   gcc/cp/pt.cc                                  | 21 +++++++++++--------
>   .../g++.dg/cpp1z/class-deduction111.C         | 10 +++++++++
>   2 files changed, 22 insertions(+), 9 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction111.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 28edc6ae988..eeae867a816 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -10089,16 +10089,19 @@ lookup_template_class (tree d1, tree arglist, tree in_decl, tree context,
>   	    {
>   	      context = tsubst_aggr_type (context, arglist,
>   					  complain, in_decl, true);
> -	      context = complete_type (context);
> -	      if (is_dependent_type && arg_depth > 1)
> +	      /* Try completing the enclosing context if it's not already so.  */
> +	      if (context != error_mark_node
> +		  && !COMPLETE_TYPE_P (context))
>   		{
> +		  context = complete_type (context);
> +		  if (COMPLETE_TYPE_P (context))
> +		    {
> +		      /* Completion could have caused us to register the desired
> +			 specialization for us, so check the table again.  */
> +		      entry = type_specializations->find_with_hash (&elt, hash);
> +		      if (entry)
> +			return entry->spec;
> +		    }
>   		}
>   	    }
>   	}
> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction111.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction111.C
> new file mode 100644
> index 00000000000..2406529ea5a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction111.C
> @@ -0,0 +1,10 @@
> +// PR c++/105982
> +// { dg-do compile { target c++17 } }
> +
> +template<class T>
> +struct A {
> +  struct B { struct C { }; };
> +  A(T, typename B::C);
> +};
> +
> +A a(0, {});