| Message ID | 20220915201627.2942314-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: modules ICE with typename friend declaration | expand |
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 > >
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); };
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,
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
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) { } +};