* [PATCH] c++ modules: uninstantiated template friend class [PR104234]
@ 2023-01-25 20:16 Patrick Palka
2023-02-03 0:56 ` Nathan Sidwell
0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2023-01-25 20:16 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, nathan, Patrick Palka
Here we're not clearing DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P for
the instantiated/injected template friend class B, which confuses a
later call to get_originating_module_decl for B. This patch fixes this
by clearing the flag in tsubst_friend_class (as is already done for
template friend functions by r11-5730-gf7aeb823d9b0de).
After fixing that, we still fail to compile the testcase, rejecting the
later definition of B with
friend-6_a.C:10:26: error: cannot declare ‘struct B<T>’ in a different module
ultimately because DECL_MODULE_ATTACH_P wasn't set on the original
(injected) declaration of B. This patch fixes this by calling
set_originating_module in tsubst_friend_class, but for that to work it
seems we need to relax the assert in this latter function since
get_originating_module_decl when called on the TYPE_DECL for B returns
the corresponding TEMPLATE_DECL.
(Alternatively we can instead call set_originating_module on the
TYPE_DECL B as soon as it's created in lookup_template_class (which is
what pushtag does), which doesn't need this assert change because at
this point the TYPE_DECL doesn't have any TEMPLATE_INFO so
get_originating_module_decl becomes a no-op. Would that be preferable?)
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
PR c++/104234
gcc/cp/ChangeLog:
* module.cc (set_originating_module): Document default argument.
Relax assert to look through DECL_TEMPLATE_RESULT in the result
of get_originating_module_decl.
* pt.cc (tsubst_friend_class): Clear
DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P and call
set_originating_module on the instantiated template friend class.
gcc/testsuite/ChangeLog:
* g++.dg/modules/friend-6_a.C: New test.
---
gcc/cp/module.cc | 8 ++++++--
gcc/cp/pt.cc | 3 +++
gcc/testsuite/g++.dg/modules/friend-6_a.C | 10 ++++++++++
3 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7133009dba5..234ce43b70f 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18843,14 +18843,18 @@ set_defining_module (tree decl)
}
void
-set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED)
+set_originating_module (tree decl, bool friend_p /* = false */)
{
set_instantiating_module (decl);
if (!DECL_NAMESPACE_SCOPE_P (decl))
return;
- gcc_checking_assert (friend_p || decl == get_originating_module_decl (decl));
+ if (!friend_p)
+ {
+ tree o = get_originating_module_decl (decl);
+ gcc_checking_assert (STRIP_TEMPLATE (o) == decl);
+ }
if (module_attach_p ())
{
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index cbe5898b553..f2ee74025e7 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11520,6 +11520,9 @@ tsubst_friend_class (tree friend_tmpl, tree args)
CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl))
= INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl)));
+ DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (tmpl) = false;
+ set_originating_module (DECL_TEMPLATE_RESULT (tmpl));
+
/* Substitute into and set the constraints on the new declaration. */
if (tree ci = get_constraints (friend_tmpl))
{
diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C
new file mode 100644
index 00000000000..97017e4ee78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C
@@ -0,0 +1,10 @@
+// PR c++/104234
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi pr104234 }
+export module pr104234;
+
+template<class> struct A {
+ template<class T> friend struct B;
+};
+A<int> a;
+template<class T> struct B { };
--
2.39.1.348.g5dec958dcf
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] c++ modules: uninstantiated template friend class [PR104234]
2023-01-25 20:16 [PATCH] c++ modules: uninstantiated template friend class [PR104234] Patrick Palka
@ 2023-02-03 0:56 ` Nathan Sidwell
0 siblings, 0 replies; 2+ messages in thread
From: Nathan Sidwell @ 2023-02-03 0:56 UTC (permalink / raw)
To: Patrick Palka, gcc-patches; +Cc: jason
That might be sufficient for this case, but temploid friends violate an
assumption of the implementation -- namely that module A cannot create an entity
that belongs in module B's symbol table. This causes a bunch of excitement,
particularly around handling (well formed) duplicatd instantions.
I'm not sure of the way to handle that, but I suspect something along the lines
of a flag on such decls and a new hash table to hold these exceptions.
nathan
On 1/25/23 15:16, Patrick Palka wrote:
> Here we're not clearing DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P for
> the instantiated/injected template friend class B, which confuses a
> later call to get_originating_module_decl for B. This patch fixes this
> by clearing the flag in tsubst_friend_class (as is already done for
> template friend functions by r11-5730-gf7aeb823d9b0de).
>
> After fixing that, we still fail to compile the testcase, rejecting the
> later definition of B with
>
> friend-6_a.C:10:26: error: cannot declare ‘struct B<T>’ in a different module
>
> ultimately because DECL_MODULE_ATTACH_P wasn't set on the original
> (injected) declaration of B. This patch fixes this by calling
> set_originating_module in tsubst_friend_class, but for that to work it
> seems we need to relax the assert in this latter function since
> get_originating_module_decl when called on the TYPE_DECL for B returns
> the corresponding TEMPLATE_DECL.
>
> (Alternatively we can instead call set_originating_module on the
> TYPE_DECL B as soon as it's created in lookup_template_class (which is
> what pushtag does), which doesn't need this assert change because at
> this point the TYPE_DECL doesn't have any TEMPLATE_INFO so
> get_originating_module_decl becomes a no-op. Would that be preferable?)
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
>
> PR c++/104234
>
> gcc/cp/ChangeLog:
>
> * module.cc (set_originating_module): Document default argument.
> Relax assert to look through DECL_TEMPLATE_RESULT in the result
> of get_originating_module_decl.
> * pt.cc (tsubst_friend_class): Clear
> DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P and call
> set_originating_module on the instantiated template friend class.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/friend-6_a.C: New test.
> ---
> gcc/cp/module.cc | 8 ++++++--
> gcc/cp/pt.cc | 3 +++
> gcc/testsuite/g++.dg/modules/friend-6_a.C | 10 ++++++++++
> 3 files changed, 19 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 7133009dba5..234ce43b70f 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -18843,14 +18843,18 @@ set_defining_module (tree decl)
> }
>
> void
> -set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED)
> +set_originating_module (tree decl, bool friend_p /* = false */)
> {
> set_instantiating_module (decl);
>
> if (!DECL_NAMESPACE_SCOPE_P (decl))
> return;
>
> - gcc_checking_assert (friend_p || decl == get_originating_module_decl (decl));
> + if (!friend_p)
> + {
> + tree o = get_originating_module_decl (decl);
> + gcc_checking_assert (STRIP_TEMPLATE (o) == decl);
> + }
>
> if (module_attach_p ())
> {
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index cbe5898b553..f2ee74025e7 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -11520,6 +11520,9 @@ tsubst_friend_class (tree friend_tmpl, tree args)
> CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl))
> = INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl)));
>
> + DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (tmpl) = false;
> + set_originating_module (DECL_TEMPLATE_RESULT (tmpl));
> +
> /* Substitute into and set the constraints on the new declaration. */
> if (tree ci = get_constraints (friend_tmpl))
> {
> diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C
> new file mode 100644
> index 00000000000..97017e4ee78
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C
> @@ -0,0 +1,10 @@
> +// PR c++/104234
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi pr104234 }
> +export module pr104234;
> +
> +template<class> struct A {
> + template<class T> friend struct B;
> +};
> +A<int> a;
> +template<class T> struct B { };
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-02-03 0:56 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 20:16 [PATCH] c++ modules: uninstantiated template friend class [PR104234] Patrick Palka
2023-02-03 0:56 ` Nathan Sidwell
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).