From: Jason Merrill <jason@redhat.com>
To: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: constrained partial spec forward decl [PR96363]
Date: Thu, 26 May 2022 09:23:51 -0400 [thread overview]
Message-ID: <6f12779b-82fe-b00f-fb35-d026e157288e@redhat.com> (raw)
In-Reply-To: <20220525172411.1922336-1-ppalka@redhat.com>
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);
prev parent reply other threads:[~2022-05-26 13:24 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-25 17:24 Patrick Palka
2022-05-26 13:23 ` Jason Merrill [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6f12779b-82fe-b00f-fb35-d026e157288e@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ppalka@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).