c++: constrained partial spec forward decl [PR96363]

Message ID 20220525172411.1922336-1-ppalka@redhat.com
State New
Headers
Series c++: constrained partial spec forward decl [PR96363] |

Commit Message

Patrick Palka May 25, 2022, 5:24 p.m. UTC
  Here during cp_parser_single_declaration for #2, we were calling
associate_classtype_constraints for TPL<T> (the primary template type)
before maybe_process_partial_specialization could get a chance to
notice that we're in fact declaring a distinct constrained partial
spec and not redeclaring the primary template.  This caused us to
emit a bogus error about differing constraints b/t the primary template
the current constraints at #2.  This patch fixes this by moving the
call to associate_classtype_constraints after the call to shadow_tag
(which calls maybe_process_partial_specialization) and adjusting
shadow_tag to use the return value of m_p_p_s.

Moreover, if we later try to define a constrained partial specialization
that's been declared earlier (as in the third testcase), then
maybe_new_partial_specialization correctly notices that it's a
redeclaration and returns NULL_TREE.  But we need to also update TYPE to
point to the constrained class type in this case (it'll otherwise
continue to point to the primary template type, eventually leading to a
bogus error).

Bootstrapped and regtested on x86_64-pc-linux-gnu, also tested against
cmcstl and range-v3, does this look OK for trunk?  Since it should only
affect concepts code, I wonder about backporting this for 12.2?

	PR c++/96363

gcc/cp/ChangeLog:

	* decl.cc (shadow_tag): Use the return value of
	maybe_process_partial_specialization.
	* parser.cc (cp_parser_single_declaration): Call shadow_tag
	before associate_classtype_constraints.
	* pt.cc (maybe_new_partial_specialization): Change return type
	to bool.  Take 'type' argument by mutable reference.  Set 'type'
	to point to the correct constrained specialization when
	appropriate.
	(maybe_process_partial_specialization): Adjust accordingly.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-partial-spec12.C: New test.
	* g++.dg/cpp2a/concepts-partial-spec13.C: New test.
---
 gcc/cp/decl.cc                                |  3 +-
 gcc/cp/parser.cc                              | 12 +++---
 gcc/cp/pt.cc                                  | 38 ++++++++++---------
 .../g++.dg/cpp2a/concepts-partial-spec12.C    | 10 +++++
 .../g++.dg/cpp2a/concepts-partial-spec12a.C   | 14 +++++++
 .../g++.dg/cpp2a/concepts-partial-spec13.C    | 16 ++++++++
 6 files changed, 69 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C
  

Comments

Jason Merrill May 26, 2022, 1:23 p.m. UTC | #1
On 5/25/22 13:24, Patrick Palka wrote:
> Here during cp_parser_single_declaration for #2, we were calling
> associate_classtype_constraints for TPL<T> (the primary template type)
> before maybe_process_partial_specialization could get a chance to
> notice that we're in fact declaring a distinct constrained partial
> spec and not redeclaring the primary template.  This caused us to
> emit a bogus error about differing constraints b/t the primary template
> the current constraints at #2.  This patch fixes this by moving the
> call to associate_classtype_constraints after the call to shadow_tag
> (which calls maybe_process_partial_specialization) and adjusting
> shadow_tag to use the return value of m_p_p_s.
> 
> Moreover, if we later try to define a constrained partial specialization
> that's been declared earlier (as in the third testcase), then
> maybe_new_partial_specialization correctly notices that it's a
> redeclaration and returns NULL_TREE.  But we need to also update TYPE to
> point to the constrained class type in this case (it'll otherwise
> continue to point to the primary template type, eventually leading to a
> bogus error).
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, also tested against
> cmcstl and range-v3, does this look OK for trunk?

OK.

> Since it should only
> affect concepts code, I wonder about backporting this for 12.2?

OK.

> 	PR c++/96363
> 
> gcc/cp/ChangeLog:
> 
> 	* decl.cc (shadow_tag): Use the return value of
> 	maybe_process_partial_specialization.
> 	* parser.cc (cp_parser_single_declaration): Call shadow_tag
> 	before associate_classtype_constraints.
> 	* pt.cc (maybe_new_partial_specialization): Change return type
> 	to bool.  Take 'type' argument by mutable reference.  Set 'type'
> 	to point to the correct constrained specialization when
> 	appropriate.
> 	(maybe_process_partial_specialization): Adjust accordingly.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-partial-spec12.C: New test.
> 	* g++.dg/cpp2a/concepts-partial-spec13.C: New test.
> ---
>   gcc/cp/decl.cc                                |  3 +-
>   gcc/cp/parser.cc                              | 12 +++---
>   gcc/cp/pt.cc                                  | 38 ++++++++++---------
>   .../g++.dg/cpp2a/concepts-partial-spec12.C    | 10 +++++
>   .../g++.dg/cpp2a/concepts-partial-spec12a.C   | 14 +++++++
>   .../g++.dg/cpp2a/concepts-partial-spec13.C    | 16 ++++++++
>   6 files changed, 69 insertions(+), 24 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 381259cb9cf..c7caa12f061 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -5464,7 +5464,8 @@ shadow_tag (cp_decl_specifier_seq *declspecs)
>     if (!t)
>       return NULL_TREE;
>   
> -  if (maybe_process_partial_specialization (t) == error_mark_node)
> +  t = maybe_process_partial_specialization (t);
> +  if (t == error_mark_node)
>       return NULL_TREE;
>   
>     /* This is where the variables in an anonymous union are
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 868b8610d60..d9e78e1f4cc 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -31811,12 +31811,6 @@ cp_parser_single_declaration (cp_parser* parser,
>         if (cp_parser_declares_only_class_p (parser)
>   	  || (declares_class_or_enum & 2))
>   	{
> -	  /* If this is a declaration, but not a definition, associate
> -	     any constraints with the type declaration. Constraints
> -	     are associated with definitions in cp_parser_class_specifier.  */
> -	  if (declares_class_or_enum == 1)
> -	    associate_classtype_constraints (decl_specifiers.type);
> -
>   	  decl = shadow_tag (&decl_specifiers);
>   
>   	  /* In this case:
> @@ -31838,6 +31832,12 @@ cp_parser_single_declaration (cp_parser* parser,
>   	  else
>   	    decl = error_mark_node;
>   
> +	  /* If this is a declaration, but not a definition, associate
> +	     any constraints with the type declaration. Constraints
> +	     are associated with definitions in cp_parser_class_specifier.  */
> +	  if (declares_class_or_enum == 1)
> +	    associate_classtype_constraints (TREE_TYPE (decl));
> +
>   	  /* Perform access checks for template parameters.  */
>   	  cp_parser_perform_template_parameter_access_checks (checks);
>   
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index b45a29926d2..7de9b11bd12 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -874,12 +874,12 @@ check_explicit_instantiation_namespace (tree spec)
>   	       spec, current_namespace, ns);
>   }
>   
> -/* Returns the type of a template specialization only if that
> -   specialization needs to be defined. Otherwise (e.g., if the type has
> -   already been defined), the function returns NULL_TREE.  */
> +/* Returns true if TYPE is a new partial specialization that needs to be
> +   set up.  This may also modify TYPE to point to the correct (new or
> +   existing) constrained partial specialization in any case.  */
>   
> -static tree
> -maybe_new_partial_specialization (tree type)
> +static bool
> +maybe_new_partial_specialization (tree& type)
>   {
>     /* An implicit instantiation of an incomplete type implies
>        the definition of a new class template.
> @@ -893,7 +893,7 @@ maybe_new_partial_specialization (tree type)
>        Here, S<T*> is an implicit instantiation of S whose type
>        is incomplete.  */
>     if (CLASSTYPE_IMPLICIT_INSTANTIATION (type) && !COMPLETE_TYPE_P (type))
> -    return type;
> +    return true;
>   
>     /* It can also be the case that TYPE is a completed specialization.
>        Continuing the previous example, suppose we also declare:
> @@ -919,11 +919,11 @@ maybe_new_partial_specialization (tree type)
>         /* If there are no template parameters, this cannot be a new
>   	 partial template specialization?  */
>         if (!current_template_parms)
> -        return NULL_TREE;
> +	return false;
>   
>         /* The injected-class-name is not a new partial specialization.  */
>         if (DECL_SELF_REFERENCE_P (TYPE_NAME (type)))
> -	return NULL_TREE;
> +	return false;
>   
>         /* If the constraints are not the same as those of the primary
>   	 then, we can probably create a new specialization.  */
> @@ -933,7 +933,7 @@ maybe_new_partial_specialization (tree type)
>   	{
>   	  tree main_constr = get_constraints (tmpl);
>   	  if (equivalent_constraints (type_constr, main_constr))
> -	    return NULL_TREE;
> +	    return false;
>   	}
>   
>         /* Also, if there's a pre-existing specialization with matching
> @@ -946,7 +946,10 @@ maybe_new_partial_specialization (tree type)
>             tree spec_constr = get_constraints (spec_tmpl);
>             if (comp_template_args (args, spec_args)
>   	      && equivalent_constraints (type_constr, spec_constr))
> -            return NULL_TREE;
> +	    {
> +	      type = TREE_TYPE (spec_tmpl);
> +	      return false;
> +	    }
>             specs = TREE_CHAIN (specs);
>           }
>   
> @@ -971,10 +974,11 @@ maybe_new_partial_specialization (tree type)
>         set_instantiating_module (d);
>         DECL_MODULE_EXPORT_P (d) = DECL_MODULE_EXPORT_P (tmpl);
>   
> -      return t;
> +      type = t;
> +      return true;
>       }
>   
> -  return NULL_TREE;
> +  return false;
>   }
>   
>   /* The TYPE is being declared.  If it is a template type, that means it
> @@ -1030,16 +1034,16 @@ maybe_process_partial_specialization (tree type)
>   
>   	 Make sure that `C<int>' and `C<T*>' are implicit instantiations.  */
>   
> -      if (tree t = maybe_new_partial_specialization (type))
> +      if (maybe_new_partial_specialization (type))
>   	{
> -	  if (!check_specialization_namespace (CLASSTYPE_TI_TEMPLATE (t))
> +	  if (!check_specialization_namespace (CLASSTYPE_TI_TEMPLATE (type))
>   	      && !at_namespace_scope_p ())
>   	    return error_mark_node;
> -	  SET_CLASSTYPE_TEMPLATE_SPECIALIZATION (t);
> -	  DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (t)) = input_location;
> +	  SET_CLASSTYPE_TEMPLATE_SPECIALIZATION (type);
> +	  DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)) = input_location;
>   	  if (processing_template_decl)
>   	    {
> -	      tree decl = push_template_decl (TYPE_MAIN_DECL (t));
> +	      tree decl = push_template_decl (TYPE_MAIN_DECL (type));
>   	      if (decl == error_mark_node)
>   		return error_mark_node;
>   	      return TREE_TYPE (decl);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
> new file mode 100644
> index 00000000000..7868092af2b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
> @@ -0,0 +1,10 @@
> +// PR c++/96363
> +// { dg-do compile { target c++20 } }
> +
> +template<class T> class TPL;
> +
> +template<class T> requires true class TPL<T>;    // #1
> +template<class T> requires false class TPL<T>;   // #2 error here
> +
> +template<class T> requires true class TPL<T*>;   // #1
> +template<class T> requires false class TPL<T*>;  // #2 error here
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C
> new file mode 100644
> index 00000000000..18e67f70944
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C
> @@ -0,0 +1,14 @@
> +// PR c++/96363
> +// { dg-do compile { target c++20 } }
> +// A version of concepts-partial-spec12.C where the primary template is
> +// constrained.
> +
> +template<class T> concept C = true;
> +
> +template<C T> class TPL;
> +
> +template<C T> requires true class TPL<T>;    // #1
> +template<C T> requires false class TPL<T>;   // #2 error here
> +
> +template<C T> requires true class TPL<T*>;   // #1
> +template<C T> requires false class TPL<T*>;  // #2 error here
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C
> new file mode 100644
> index 00000000000..78f6906b1ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C
> @@ -0,0 +1,16 @@
> +// PR c++/99501
> +// { dg-do compile { target c++20 } }
> +
> +template<auto> struct X{};
> +
> +template<auto V> requires requires{V.a;} struct X<V>;
> +template<auto V> requires requires{V.b;} struct X<V>;
> +
> +template<auto V> requires requires{V.a;} struct X<V> { static const bool v = false; };
> +template<auto V> requires requires{V.b;} struct X<V> { static const bool v = true; };
> +
> +struct A  {int a; };
> +static_assert(!X<A{}>::v);
> +
> +struct B { int b; };
> +static_assert(X<B{}>::v);
  

Patch

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 381259cb9cf..c7caa12f061 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -5464,7 +5464,8 @@  shadow_tag (cp_decl_specifier_seq *declspecs)
   if (!t)
     return NULL_TREE;
 
-  if (maybe_process_partial_specialization (t) == error_mark_node)
+  t = maybe_process_partial_specialization (t);
+  if (t == error_mark_node)
     return NULL_TREE;
 
   /* This is where the variables in an anonymous union are
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 868b8610d60..d9e78e1f4cc 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -31811,12 +31811,6 @@  cp_parser_single_declaration (cp_parser* parser,
       if (cp_parser_declares_only_class_p (parser)
 	  || (declares_class_or_enum & 2))
 	{
-	  /* If this is a declaration, but not a definition, associate
-	     any constraints with the type declaration. Constraints
-	     are associated with definitions in cp_parser_class_specifier.  */
-	  if (declares_class_or_enum == 1)
-	    associate_classtype_constraints (decl_specifiers.type);
-
 	  decl = shadow_tag (&decl_specifiers);
 
 	  /* In this case:
@@ -31838,6 +31832,12 @@  cp_parser_single_declaration (cp_parser* parser,
 	  else
 	    decl = error_mark_node;
 
+	  /* If this is a declaration, but not a definition, associate
+	     any constraints with the type declaration. Constraints
+	     are associated with definitions in cp_parser_class_specifier.  */
+	  if (declares_class_or_enum == 1)
+	    associate_classtype_constraints (TREE_TYPE (decl));
+
 	  /* Perform access checks for template parameters.  */
 	  cp_parser_perform_template_parameter_access_checks (checks);
 
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index b45a29926d2..7de9b11bd12 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -874,12 +874,12 @@  check_explicit_instantiation_namespace (tree spec)
 	       spec, current_namespace, ns);
 }
 
-/* Returns the type of a template specialization only if that
-   specialization needs to be defined. Otherwise (e.g., if the type has
-   already been defined), the function returns NULL_TREE.  */
+/* Returns true if TYPE is a new partial specialization that needs to be
+   set up.  This may also modify TYPE to point to the correct (new or
+   existing) constrained partial specialization in any case.  */
 
-static tree
-maybe_new_partial_specialization (tree type)
+static bool
+maybe_new_partial_specialization (tree& type)
 {
   /* An implicit instantiation of an incomplete type implies
      the definition of a new class template.
@@ -893,7 +893,7 @@  maybe_new_partial_specialization (tree type)
      Here, S<T*> is an implicit instantiation of S whose type
      is incomplete.  */
   if (CLASSTYPE_IMPLICIT_INSTANTIATION (type) && !COMPLETE_TYPE_P (type))
-    return type;
+    return true;
 
   /* It can also be the case that TYPE is a completed specialization.
      Continuing the previous example, suppose we also declare:
@@ -919,11 +919,11 @@  maybe_new_partial_specialization (tree type)
       /* If there are no template parameters, this cannot be a new
 	 partial template specialization?  */
       if (!current_template_parms)
-        return NULL_TREE;
+	return false;
 
       /* The injected-class-name is not a new partial specialization.  */
       if (DECL_SELF_REFERENCE_P (TYPE_NAME (type)))
-	return NULL_TREE;
+	return false;
 
       /* If the constraints are not the same as those of the primary
 	 then, we can probably create a new specialization.  */
@@ -933,7 +933,7 @@  maybe_new_partial_specialization (tree type)
 	{
 	  tree main_constr = get_constraints (tmpl);
 	  if (equivalent_constraints (type_constr, main_constr))
-	    return NULL_TREE;
+	    return false;
 	}
 
       /* Also, if there's a pre-existing specialization with matching
@@ -946,7 +946,10 @@  maybe_new_partial_specialization (tree type)
           tree spec_constr = get_constraints (spec_tmpl);
           if (comp_template_args (args, spec_args)
 	      && equivalent_constraints (type_constr, spec_constr))
-            return NULL_TREE;
+	    {
+	      type = TREE_TYPE (spec_tmpl);
+	      return false;
+	    }
           specs = TREE_CHAIN (specs);
         }
 
@@ -971,10 +974,11 @@  maybe_new_partial_specialization (tree type)
       set_instantiating_module (d);
       DECL_MODULE_EXPORT_P (d) = DECL_MODULE_EXPORT_P (tmpl);
 
-      return t;
+      type = t;
+      return true;
     }
 
-  return NULL_TREE;
+  return false;
 }
 
 /* The TYPE is being declared.  If it is a template type, that means it
@@ -1030,16 +1034,16 @@  maybe_process_partial_specialization (tree type)
 
 	 Make sure that `C<int>' and `C<T*>' are implicit instantiations.  */
 
-      if (tree t = maybe_new_partial_specialization (type))
+      if (maybe_new_partial_specialization (type))
 	{
-	  if (!check_specialization_namespace (CLASSTYPE_TI_TEMPLATE (t))
+	  if (!check_specialization_namespace (CLASSTYPE_TI_TEMPLATE (type))
 	      && !at_namespace_scope_p ())
 	    return error_mark_node;
-	  SET_CLASSTYPE_TEMPLATE_SPECIALIZATION (t);
-	  DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (t)) = input_location;
+	  SET_CLASSTYPE_TEMPLATE_SPECIALIZATION (type);
+	  DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (type)) = input_location;
 	  if (processing_template_decl)
 	    {
-	      tree decl = push_template_decl (TYPE_MAIN_DECL (t));
+	      tree decl = push_template_decl (TYPE_MAIN_DECL (type));
 	      if (decl == error_mark_node)
 		return error_mark_node;
 	      return TREE_TYPE (decl);
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
new file mode 100644
index 00000000000..7868092af2b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12.C
@@ -0,0 +1,10 @@ 
+// PR c++/96363
+// { dg-do compile { target c++20 } }
+
+template<class T> class TPL;
+
+template<class T> requires true class TPL<T>;    // #1
+template<class T> requires false class TPL<T>;   // #2 error here
+
+template<class T> requires true class TPL<T*>;   // #1
+template<class T> requires false class TPL<T*>;  // #2 error here
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C
new file mode 100644
index 00000000000..18e67f70944
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec12a.C
@@ -0,0 +1,14 @@ 
+// PR c++/96363
+// { dg-do compile { target c++20 } }
+// A version of concepts-partial-spec12.C where the primary template is
+// constrained.
+
+template<class T> concept C = true;
+
+template<C T> class TPL;
+
+template<C T> requires true class TPL<T>;    // #1
+template<C T> requires false class TPL<T>;   // #2 error here
+
+template<C T> requires true class TPL<T*>;   // #1
+template<C T> requires false class TPL<T*>;  // #2 error here
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C
new file mode 100644
index 00000000000..78f6906b1ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec13.C
@@ -0,0 +1,16 @@ 
+// PR c++/99501
+// { dg-do compile { target c++20 } }
+
+template<auto> struct X{};
+
+template<auto V> requires requires{V.a;} struct X<V>;
+template<auto V> requires requires{V.b;} struct X<V>;
+
+template<auto V> requires requires{V.a;} struct X<V> { static const bool v = false; };
+template<auto V> requires requires{V.b;} struct X<V> { static const bool v = true; };
+
+struct A  {int a; };
+static_assert(!X<A{}>::v);
+
+struct B { int b; };
+static_assert(X<B{}>::v);