From: Nathan Sidwell <nathan@acm.org>
To: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Cc: jason@redhat.com
Subject: Re: [PATCH] c++ modules: variable template partial spec fixes [PR107033]
Date: Mon, 26 Sep 2022 10:47:47 -0400 [thread overview]
Message-ID: <9dd2a7d3-4800-6db5-e678-3273755866ac@acm.org> (raw)
In-Reply-To: <20220926143620.24037-1-ppalka@redhat.com>
On 9/26/22 10:36, Patrick Palka wrote:
> In r13-2775-g32d8123cd6ce87 I overlooked that we need to adjust the
> call to add_mergeable_specialization in the MK_partial case to correctly
> handle variable template partial specializations (it currently assumes
> we're always dealing with one for a class template). This fixes an ICE
> when converting the testcase from that commit to use an importable
> header instead of a named module.
looks good, thanks
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> PR c++/107033
>
> gcc/cp/ChangeLog:
>
> * module.cc (trees_in::decl_value): In the MK_partial case
> for a variable template partial specialization, pass decl_p=true
> to add_mergeable_specialization and set spec to the VAR_DECL
> not the TEMPLATE_DECL.
> * pt.cc (add_mergeable_specialization): For a variable template
> partial specialization, set the TREE_TYPE of the new
> DECL_TEMPLATE_SPECIALIZATIONS node to the TREE_TYPE of the
> VAR_DECL not the VAR_DECL itself.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/partial-2.cc, g++.dg/modules/partial-2.h: New
> files, factored out from ...
> * g++.dg/modules/partial-2_a.C, g++.dg/modules/partial-2_b.C: ...
> here.
> * g++.dg/modules/partial-2_c.H: New test.
> * g++.dg/modules/partial-2_d.C: New test.
> ---
> gcc/cp/module.cc | 17 ++++++----
> gcc/cp/pt.cc | 2 +-
> gcc/testsuite/g++.dg/modules/partial-2.cc | 17 ++++++++++
> gcc/testsuite/g++.dg/modules/partial-2.h | 38 +++++++++++++++++++++
> gcc/testsuite/g++.dg/modules/partial-2_a.C | 39 +---------------------
> gcc/testsuite/g++.dg/modules/partial-2_b.C | 18 +---------
> gcc/testsuite/g++.dg/modules/partial-2_c.H | 5 +++
> gcc/testsuite/g++.dg/modules/partial-2_d.C | 8 +++++
> 8 files changed, 82 insertions(+), 62 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.cc
> create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.h
> create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_c.H
> create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_d.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f23832cb56a..7496df5e843 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -8185,13 +8185,18 @@ trees_in::decl_value ()
> /* Set the TEMPLATE_DECL's type. */
> TREE_TYPE (decl) = TREE_TYPE (inner);
>
> - if (mk & MK_template_mask
> - || mk == MK_partial)
> + /* Add to specialization tables now that constraints etc are
> + added. */
> + if (mk == MK_partial)
> {
> - /* Add to specialization tables now that constraints etc are
> - added. */
> - bool is_type = mk == MK_partial || !(mk & MK_tmpl_decl_mask);
> -
> + bool is_type = TREE_CODE (inner) == TYPE_DECL;
> + spec.spec = is_type ? type : inner;
> + add_mergeable_specialization (!is_type, false,
> + &spec, decl, spec_flags);
> + }
> + else if (mk & MK_template_mask)
> + {
> + bool is_type = !(mk & MK_tmpl_decl_mask);
> spec.spec = is_type ? type : mk & MK_tmpl_tmpl_mask ? inner : decl;
> add_mergeable_specialization (!is_type,
> !is_type && mk & MK_tmpl_alias_mask,
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index db4e808adec..1f088fe281e 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -31010,7 +31010,7 @@ add_mergeable_specialization (bool decl_p, bool alias_p, spec_entry *elt,
> /* A partial specialization. */
> tree cons = tree_cons (elt->args, decl,
> DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl));
> - TREE_TYPE (cons) = elt->spec;
> + TREE_TYPE (cons) = decl_p ? TREE_TYPE (elt->spec) : elt->spec;
> DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl) = cons;
> }
> }
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2.cc b/gcc/testsuite/g++.dg/modules/partial-2.cc
> new file mode 100644
> index 00000000000..1316bf5e1c5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2.cc
> @@ -0,0 +1,17 @@
> +static_assert(is_reference_v<int&>);
> +static_assert(is_reference_v<int&&>);
> +static_assert(!is_reference_v<int>);
> +
> +static_assert(A::is_reference_v<long&>);
> +static_assert(A::is_reference_v<long&&>);
> +static_assert(!A::is_reference_v<long>);
> +
> +#if __cpp_concepts
> +static_assert(concepts::is_reference_v<char&>);
> +static_assert(concepts::is_reference_v<char&&>);
> +static_assert(!concepts::is_reference_v<char>);
> +
> +static_assert(concepts::A::is_reference_v<bool&>);
> +static_assert(concepts::A::is_reference_v<bool&&>);
> +static_assert(!concepts::A::is_reference_v<bool>);
> +#endif
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2.h b/gcc/testsuite/g++.dg/modules/partial-2.h
> new file mode 100644
> index 00000000000..afcfce791b3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2.h
> @@ -0,0 +1,38 @@
> +template<class T> constexpr bool is_reference_v = false;
> +template<class T> constexpr bool is_reference_v<T&> = true;
> +template<class T> constexpr bool is_reference_v<T&&> = true;
> +
> +struct A {
> + template<class T> static constexpr bool is_reference_v = false;
> +};
> +
> +template<class T> constexpr bool A::is_reference_v<T&> = true;
> +template<class T> constexpr bool A::is_reference_v<T&&> = true;
> +
> +#if __cpp_concepts
> +namespace concepts {
> + template<class T> bool is_reference_v;
> +
> + template<class T> requires __is_same(T, T&)
> + constexpr bool is_reference_v<T> = true;
> +
> + template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> + constexpr bool is_reference_v<T> = true;
> +
> + template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> + constexpr bool is_reference_v<T> = false;
> +
> + struct A {
> + template<class T> static bool is_reference_v;
> + };
> +
> + template<class T> requires __is_same(T, T&)
> + constexpr bool A::is_reference_v<T> = true;
> +
> + template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> + constexpr bool A::is_reference_v<T> = true;
> +
> + template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> + constexpr bool A::is_reference_v<T> = false;
> +}
> +#endif
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_a.C b/gcc/testsuite/g++.dg/modules/partial-2_a.C
> index 2681bb59ce8..1582f56f2d4 100644
> --- a/gcc/testsuite/g++.dg/modules/partial-2_a.C
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_a.C
> @@ -3,41 +3,4 @@
> // { dg-module-cmi pr106826 }
> export module pr106826;
>
> -template<class T> constexpr bool is_reference_v = false;
> -template<class T> constexpr bool is_reference_v<T&> = true;
> -template<class T> constexpr bool is_reference_v<T&&> = true;
> -
> -struct A {
> - template<class T> static constexpr bool is_reference_v = false;
> -};
> -
> -template<class T> constexpr bool A::is_reference_v<T&> = true;
> -template<class T> constexpr bool A::is_reference_v<T&&> = true;
> -
> -#if __cpp_concepts
> -namespace concepts {
> - template<class T> bool is_reference_v;
> -
> - template<class T> requires __is_same(T, T&)
> - constexpr bool is_reference_v<T> = true;
> -
> - template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> - constexpr bool is_reference_v<T> = true;
> -
> - template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> - constexpr bool is_reference_v<T> = false;
> -
> - struct A {
> - template<class T> static bool is_reference_v;
> - };
> -
> - template<class T> requires __is_same(T, T&)
> - constexpr bool A::is_reference_v<T> = true;
> -
> - template<class T> requires __is_same(T, T&&) && (!__is_same(T, T&))
> - constexpr bool A::is_reference_v<T> = true;
> -
> - template<class T> requires (!__is_same(T, T&)) && (!__is_same(T, T&&))
> - constexpr bool A::is_reference_v<T> = false;
> -}
> -#endif
> +#include "partial-2.h"
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_b.C b/gcc/testsuite/g++.dg/modules/partial-2_b.C
> index 0af41ef5e5e..1b0c7a53e9f 100644
> --- a/gcc/testsuite/g++.dg/modules/partial-2_b.C
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_b.C
> @@ -2,20 +2,4 @@
> // { dg-additional-options -fmodules-ts }
> module pr106826;
>
> -static_assert(is_reference_v<int&>);
> -static_assert(is_reference_v<int&&>);
> -static_assert(!is_reference_v<int>);
> -
> -static_assert(A::is_reference_v<long&>);
> -static_assert(A::is_reference_v<long&&>);
> -static_assert(!A::is_reference_v<long>);
> -
> -#if __cpp_concepts
> -static_assert(concepts::is_reference_v<char&>);
> -static_assert(concepts::is_reference_v<char&&>);
> -static_assert(!concepts::is_reference_v<char>);
> -
> -static_assert(concepts::A::is_reference_v<bool&>);
> -static_assert(concepts::A::is_reference_v<bool&&>);
> -static_assert(!concepts::A::is_reference_v<bool>);
> -#endif
> +#include "partial-2.cc"
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_c.H b/gcc/testsuite/g++.dg/modules/partial-2_c.H
> new file mode 100644
> index 00000000000..bd838529ce0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_c.H
> @@ -0,0 +1,5 @@
> +// PR c++/107033
> +// { dg-additional-options -fmodule-header }
> +// { dg-module-cmi {} }
> +
> +#include "partial-2.h"
> diff --git a/gcc/testsuite/g++.dg/modules/partial-2_d.C b/gcc/testsuite/g++.dg/modules/partial-2_d.C
> new file mode 100644
> index 00000000000..ed54d3c2884
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/partial-2_d.C
> @@ -0,0 +1,8 @@
> +// PR c++/107033
> +// { dg-additional-options -fmodules-ts }
> +// { dg-module-cmi pr107033 }
> +export module pr107033;
> +
> +import "partial-2_c.H";
> +
> +#include "partial-2.cc"
--
Nathan Sidwell
prev parent reply other threads:[~2022-09-26 14:47 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 14:36 Patrick Palka
2022-09-26 14:47 ` Nathan Sidwell [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=9dd2a7d3-4800-6db5-e678-3273755866ac@acm.org \
--to=nathan@acm.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--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).