* [PATCH] c++: modules ICE with typename friend declaration
@ 2022-09-15 20:16 Patrick Palka
2022-09-16 7:45 ` Nathan Sidwell
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2022-09-15 20:16 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, nathan, Patrick Palka
A couple of xtreme-header-* modules tests began ICEing in C++23 mode
ever since r13-2650-g5d84a4418aa962 introduced into <ranges> the
dependently scoped friend declaration
friend /* typename */ _OuterIter::value_type;
ultimately because the streaming code assumes a TYPE_P friend must
be a class type, but here it's a TYPENAME_TYPE, which doesn't have
a TEMPLATE_INFO or CLASSTYPE_BEFRIENDING_CLASSES. This patch tries
to correct this in a minimal way.
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
gcc/cp/ChangeLog:
* module.cc (friend_from_decl_list): Don't consider
CLASSTYPE_TEMPLATE_INFO for a TYPENAME_TYPE friend.
(trees_in::read_class_def): Don't add to
CLASSTYPE_BEFRIENDING_CLASSES for a TYPENAME_TYPE friend.
gcc/testsuite/ChangeLog:
* g++.dg/modules/typename-friend.C: New test.
---
gcc/cp/module.cc | 5 +++--
gcc/testsuite/g++.dg/modules/typename-friend.C | 9 +++++++++
2 files changed, 12 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/typename-friend.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f27f4d091e5..1a1ff5be574 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -4734,7 +4734,8 @@ friend_from_decl_list (tree frnd)
if (TYPE_P (frnd))
{
res = TYPE_NAME (frnd);
- if (CLASSTYPE_TEMPLATE_INFO (frnd))
+ if (CLASS_TYPE_P (frnd)
+ && CLASSTYPE_TEMPLATE_INFO (frnd))
tmpl = CLASSTYPE_TI_TEMPLATE (frnd);
}
else if (DECL_TEMPLATE_INFO (frnd))
@@ -12121,7 +12122,7 @@ trees_in::read_class_def (tree defn, tree maybe_template)
{
tree f = TREE_VALUE (friend_classes);
- if (TYPE_P (f))
+ if (CLASS_TYPE_P (f))
{
CLASSTYPE_BEFRIENDING_CLASSES (f)
= tree_cons (NULL_TREE, type,
diff --git a/gcc/testsuite/g++.dg/modules/typename-friend.C b/gcc/testsuite/g++.dg/modules/typename-friend.C
new file mode 100644
index 00000000000..d8faf7955c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/typename-friend.C
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodules-ts" }
+
+export module x;
+
+template<class T>
+struct A {
+ friend typename T::type;
+ friend void f(A) { }
+};
--
2.37.3.662.g36f8e7ed7d
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: modules ICE with typename friend declaration
2022-09-15 20:16 [PATCH] c++: modules ICE with typename friend declaration Patrick Palka
@ 2022-09-16 7:45 ` Nathan Sidwell
2022-09-16 15:54 ` Patrick Palka
0 siblings, 1 reply; 5+ messages in thread
From: Nathan Sidwell @ 2022-09-16 7:45 UTC (permalink / raw)
To: Patrick Palka; +Cc: GCC Patches, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 2874 bytes --]
Thanks, this looks right. Sigh templates can mess up ones mental invariants!
The test case should really be a foo_[ab].C kind, to test both sides of the
streaming. Bonus points for using the template after importing. And you
need the dg-module-cmi annotation to check /and then delete/ the gcm file
produced.
nathan
On Thu, Sep 15, 2022, 22:16 Patrick Palka <ppalka@redhat.com> wrote:
> A couple of xtreme-header-* modules tests began ICEing in C++23 mode
> ever since r13-2650-g5d84a4418aa962 introduced into <ranges> the
> dependently scoped friend declaration
>
> friend /* typename */ _OuterIter::value_type;
>
> ultimately because the streaming code assumes a TYPE_P friend must
> be a class type, but here it's a TYPENAME_TYPE, which doesn't have
> a TEMPLATE_INFO or CLASSTYPE_BEFRIENDING_CLASSES. This patch tries
> to correct this in a minimal way.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> gcc/cp/ChangeLog:
>
> * module.cc (friend_from_decl_list): Don't consider
> CLASSTYPE_TEMPLATE_INFO for a TYPENAME_TYPE friend.
> (trees_in::read_class_def): Don't add to
> CLASSTYPE_BEFRIENDING_CLASSES for a TYPENAME_TYPE friend.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/typename-friend.C: New test.
> ---
> gcc/cp/module.cc | 5 +++--
> gcc/testsuite/g++.dg/modules/typename-friend.C | 9 +++++++++
> 2 files changed, 12 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/typename-friend.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f27f4d091e5..1a1ff5be574 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -4734,7 +4734,8 @@ friend_from_decl_list (tree frnd)
> if (TYPE_P (frnd))
> {
> res = TYPE_NAME (frnd);
> - if (CLASSTYPE_TEMPLATE_INFO (frnd))
> + if (CLASS_TYPE_P (frnd)
> + && CLASSTYPE_TEMPLATE_INFO (frnd))
> tmpl = CLASSTYPE_TI_TEMPLATE (frnd);
> }
> else if (DECL_TEMPLATE_INFO (frnd))
> @@ -12121,7 +12122,7 @@ trees_in::read_class_def (tree defn, tree
> maybe_template)
> {
> tree f = TREE_VALUE (friend_classes);
>
> - if (TYPE_P (f))
> + if (CLASS_TYPE_P (f))
> {
> CLASSTYPE_BEFRIENDING_CLASSES (f)
> = tree_cons (NULL_TREE, type,
> diff --git a/gcc/testsuite/g++.dg/modules/typename-friend.C
> b/gcc/testsuite/g++.dg/modules/typename-friend.C
> new file mode 100644
> index 00000000000..d8faf7955c3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/typename-friend.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +export module x;
> +
> +template<class T>
> +struct A {
> + friend typename T::type;
> + friend void f(A) { }
> +};
> --
> 2.37.3.662.g36f8e7ed7d
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: modules ICE with typename friend declaration
2022-09-16 7:45 ` Nathan Sidwell
@ 2022-09-16 15:54 ` Patrick Palka
2022-09-16 18:39 ` Bernhard Reutner-Fischer
2022-09-17 6:17 ` Nathan Sidwell
0 siblings, 2 replies; 5+ messages in thread
From: Patrick Palka @ 2022-09-16 15:54 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: Patrick Palka, GCC Patches, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 5931 bytes --]
On Fri, 16 Sep 2022, Nathan Sidwell wrote:
> Thanks, this looks right. Sigh templates can mess up ones mental invariants!
> The test case should really be a foo_[ab].C kind, to test both sides of the streaming. Bonus points for using the template after importing. And you need the dg-module-cmi annotation to check /and then
> delete/ the gcm file produced.
Aha, thanks very much for the pointers, I redid the testcase using
lang-3_[abc].C as an example. How does the following look?
-- >8 --
gcc/cp/ChangeLog:
* module.cc (friend_from_decl_list): Don't consider
CLASSTYPE_TEMPLATE_INFO for a TYPENAME_TYPE friend.
(trees_in::read_class_def): Don't add to
CLASSTYPE_BEFRIENDING_CLASSES for a TYPENAME_TYPE friend.
gcc/testsuite/ChangeLog:
* g++.dg/modules/typename-friend_a.C: New test.
* g++.dg/modules/typename-friend_b.C: New test.
---
gcc/cp/module.cc | 5 +++--
gcc/testsuite/g++.dg/modules/typename-friend_a.C | 11 +++++++++++
gcc/testsuite/g++.dg/modules/typename-friend_b.C | 6 ++++++
3 files changed, 20 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/typename-friend_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/typename-friend_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f27f4d091e5..1a1ff5be574 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -4734,7 +4734,8 @@ friend_from_decl_list (tree frnd)
if (TYPE_P (frnd))
{
res = TYPE_NAME (frnd);
- if (CLASSTYPE_TEMPLATE_INFO (frnd))
+ if (CLASS_TYPE_P (frnd)
+ && CLASSTYPE_TEMPLATE_INFO (frnd))
tmpl = CLASSTYPE_TI_TEMPLATE (frnd);
}
else if (DECL_TEMPLATE_INFO (frnd))
@@ -12121,7 +12122,7 @@ trees_in::read_class_def (tree defn, tree maybe_template)
{
tree f = TREE_VALUE (friend_classes);
- if (TYPE_P (f))
+ if (CLASS_TYPE_P (f))
{
CLASSTYPE_BEFRIENDING_CLASSES (f)
= tree_cons (NULL_TREE, type,
diff --git a/gcc/testsuite/g++.dg/modules/typename-friend_a.C b/gcc/testsuite/g++.dg/modules/typename-friend_a.C
new file mode 100644
index 00000000000..aa426fe6cf0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/typename-friend_a.C
@@ -0,0 +1,11 @@
+// { dg-additional-options "-fmodules-ts" }
+export module foo;
+// { dg-module-cmi foo }
+
+template<class T>
+struct A {
+ friend typename T::type;
+ friend void f(A) { }
+private:
+ static constexpr int value = 42;
+};
diff --git a/gcc/testsuite/g++.dg/modules/typename-friend_b.C b/gcc/testsuite/g++.dg/modules/typename-friend_b.C
new file mode 100644
index 00000000000..97da9d82e11
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/typename-friend_b.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts" }
+module foo;
+
+struct C;
+struct B { using type = C; };
+struct C { static_assert(A<B>::value == 42); };
--
2.38.0.rc0
>
> nathan
>
> On Thu, Sep 15, 2022, 22:16 Patrick Palka <ppalka@redhat.com> wrote:
> A couple of xtreme-header-* modules tests began ICEing in C++23 mode
> ever since r13-2650-g5d84a4418aa962 introduced into <ranges> the
> dependently scoped friend declaration
>
> friend /* typename */ _OuterIter::value_type;
>
> ultimately because the streaming code assumes a TYPE_P friend must
> be a class type, but here it's a TYPENAME_TYPE, which doesn't have
> a TEMPLATE_INFO or CLASSTYPE_BEFRIENDING_CLASSES. This patch tries
> to correct this in a minimal way.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> gcc/cp/ChangeLog:
>
> * module.cc (friend_from_decl_list): Don't consider
> CLASSTYPE_TEMPLATE_INFO for a TYPENAME_TYPE friend.
> (trees_in::read_class_def): Don't add to
> CLASSTYPE_BEFRIENDING_CLASSES for a TYPENAME_TYPE friend.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/typename-friend.C: New test.
> ---
> gcc/cp/module.cc | 5 +++--
> gcc/testsuite/g++.dg/modules/typename-friend.C | 9 +++++++++
> 2 files changed, 12 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/typename-friend.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f27f4d091e5..1a1ff5be574 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -4734,7 +4734,8 @@ friend_from_decl_list (tree frnd)
> if (TYPE_P (frnd))
> {
> res = TYPE_NAME (frnd);
> - if (CLASSTYPE_TEMPLATE_INFO (frnd))
> + if (CLASS_TYPE_P (frnd)
> + && CLASSTYPE_TEMPLATE_INFO (frnd))
> tmpl = CLASSTYPE_TI_TEMPLATE (frnd);
> }
> else if (DECL_TEMPLATE_INFO (frnd))
> @@ -12121,7 +12122,7 @@ trees_in::read_class_def (tree defn, tree maybe_template)
> {
> tree f = TREE_VALUE (friend_classes);
>
> - if (TYPE_P (f))
> + if (CLASS_TYPE_P (f))
> {
> CLASSTYPE_BEFRIENDING_CLASSES (f)
> = tree_cons (NULL_TREE, type,
> diff --git a/gcc/testsuite/g++.dg/modules/typename-friend.C b/gcc/testsuite/g++.dg/modules/typename-friend.C
> new file mode 100644
> index 00000000000..d8faf7955c3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/typename-friend.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +export module x;
> +
> +template<class T>
> +struct A {
> + friend typename T::type;
> + friend void f(A) { }
> +};
> --
> 2.37.3.662.g36f8e7ed7d
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: modules ICE with typename friend declaration
2022-09-16 15:54 ` Patrick Palka
@ 2022-09-16 18:39 ` Bernhard Reutner-Fischer
2022-09-17 6:17 ` Nathan Sidwell
1 sibling, 0 replies; 5+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-09-16 18:39 UTC (permalink / raw)
To: Patrick Palka, Patrick Palka via Gcc-patches, Nathan Sidwell; +Cc: GCC Patches
On 16 September 2022 17:54:32 CEST, Patrick Palka via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>+++ b/gcc/testsuite/g++.dg/modules/typename-friend_a.C
>@@ -0,0 +1,11 @@
>+// { dg-additional-options "-fmodules-ts" }
>+export module foo;
>+// { dg-module-cmi foo }
>+
If that's a constant, repeating pain, you could instrument the test suite so that upon seeing -fmodules-ts (or maybe a later std) it greps for export module and calls dg-module-cmi on those names automatically on its own.
We do the same for stuff like PCH or fortran module .mod or many other cleanup stuff because such manual annotations are IMHO a waste of developer resources..
Just a thought..
cheers,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: modules ICE with typename friend declaration
2022-09-16 15:54 ` Patrick Palka
2022-09-16 18:39 ` Bernhard Reutner-Fischer
@ 2022-09-17 6:17 ` Nathan Sidwell
1 sibling, 0 replies; 5+ messages in thread
From: Nathan Sidwell @ 2022-09-17 6:17 UTC (permalink / raw)
To: Patrick Palka; +Cc: GCC Patches, Jason Merrill
On 9/16/22 11:54, Patrick Palka wrote:
> On Fri, 16 Sep 2022, Nathan Sidwell wrote:
>
>> Thanks, this looks right. Sigh templates can mess up ones mental invariants!
>> The test case should really be a foo_[ab].C kind, to test both sides of the streaming. Bonus points for using the template after importing. And you need the dg-module-cmi annotation to check /and then
>> delete/ the gcm file produced.
>
> Aha, thanks very much for the pointers, I redid the testcase using
> lang-3_[abc].C as an example. How does the following look?
>
yes, that's right, thanks!
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-17 6:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-15 20:16 [PATCH] c++: modules ICE with typename friend declaration Patrick Palka
2022-09-16 7:45 ` Nathan Sidwell
2022-09-16 15:54 ` Patrick Palka
2022-09-16 18:39 ` Bernhard Reutner-Fischer
2022-09-17 6:17 ` 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).