* [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).