| Message ID | 661cb2d3.170a0220.c9922.173d@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: Fix instantiation of imported temploid friends [PR114275] | expand |
On Mon, 15 Apr 2024, Nathaniel Shead wrote: > I'm not a huge fan of always streaming 'imported_temploid_friends' for > all decls, but I don't think it adds much performance cost over adding a > new flag to categorise decls that might be marked as such. IIUC this value is going to be almost always null which is encoded as a single 0 byte, which should be fast to stream. But I wonder how much larger <bits/stdc++.h> gets? Can we get away with streaming this value only for TEMPLATE_DECLs? > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > -- >8 -- > > This patch fixes a number of issues with the handling of temploid friend > declarations. > > The primary issue is that instantiations of friend declarations should > attach the declaration to the same module as the befriending class, by > [module.unit] p7.1 and [temp.friend] p2; this could be a different > module from the current TU, and so needs special handling. Nice, your approach seems consistent with Nathan's comments in the past about this issue: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603288.html https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611215.htmlw > > The other main issue here is that we can't assume that just because name > lookup didn't find a definition for a hidden template class, it doesn't > mean that it doesn't exist: it could be a non-exported entity that we've > nevertheless streamed in from an imported module. We need to ensure > that when instantiating friend classes that we return the same TYPE_DECL > that we got from our imports, otherwise we will get later issues with > 'duplicate_decls' (rightfully) complaining that they're different. > > This doesn't appear necessary for functions due to the existing name > lookup handling already finding these hidden declarations. It does seem like a weird inconsistency that tsubst_friend_class needs this workaround but not tsubst_friend_function. I wonder if we can relax duplicate_decls to treat an instantiated template friend class as a redeclaration instead of complaining, mirroring its behavior for functions, which in turn would let us get rid of the name lookup in tsubst_friend_class and eliminate the need for lookup_imported_hidden_friend? This may be too speculative/risky of a refactoring at this stage though, and your approach has the nice advantage of changing only modules code paths. In any case I hope we can fix this issue for GCC 14! LGTM overall. > > PR c++/105320 > PR c++/114275 > > gcc/cp/ChangeLog: > > * cp-tree.h (propagate_defining_module): Declare. > (lookup_imported_hidden_friend): Declare. > * decl.cc (duplicate_decls): Also check if hidden declarations > can be redeclared in this module. > * module.cc (imported_temploid_friends): New map. > (init_modules): Initialize it. > (trees_out::decl_value): Write it. > (trees_in::decl_value): Read it. > (get_originating_module_decl): Follow the owning decl for an > imported temploid friend. > (propagate_defining_module): New function. > * name-lookup.cc (lookup_imported_hidden_friend): New function. > * pt.cc (tsubst_friend_function): Propagate defining module for > new friend functions. > (tsubst_friend_class): Lookup imported hidden friends. Check > for valid redeclaration. Propagate defining module for new > friend classes. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/tpl-friend-10_a.C: New test. > * g++.dg/modules/tpl-friend-10_b.C: New test. > * g++.dg/modules/tpl-friend-10_c.C: New test. > * g++.dg/modules/tpl-friend-11_a.C: New test. > * g++.dg/modules/tpl-friend-11_b.C: New test. > * g++.dg/modules/tpl-friend-12_a.C: New test. > * g++.dg/modules/tpl-friend-12_b.C: New test. > * g++.dg/modules/tpl-friend-12_c.C: New test. > * g++.dg/modules/tpl-friend-12_d.C: New test. > * g++.dg/modules/tpl-friend-12_e.C: New test. > * g++.dg/modules/tpl-friend-12_f.C: New test. > * g++.dg/modules/tpl-friend-13_a.C: New test. > * g++.dg/modules/tpl-friend-13_b.C: New test. > * g++.dg/modules/tpl-friend-13_c.C: New test. > * g++.dg/modules/tpl-friend-13_d.C: New test. > * g++.dg/modules/tpl-friend-13_e.C: New test. > * g++.dg/modules/tpl-friend-9.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/cp-tree.h | 2 + > gcc/cp/decl.cc | 36 +++++++------ > gcc/cp/module.cc | 52 +++++++++++++++++++ > gcc/cp/name-lookup.cc | 42 +++++++++++++++ > gcc/cp/pt.cc | 19 +++++++ > .../g++.dg/modules/tpl-friend-10_a.C | 15 ++++++ > .../g++.dg/modules/tpl-friend-10_b.C | 5 ++ > .../g++.dg/modules/tpl-friend-10_c.C | 7 +++ > .../g++.dg/modules/tpl-friend-11_a.C | 14 +++++ > .../g++.dg/modules/tpl-friend-11_b.C | 5 ++ > .../g++.dg/modules/tpl-friend-12_a.C | 10 ++++ > .../g++.dg/modules/tpl-friend-12_b.C | 9 ++++ > .../g++.dg/modules/tpl-friend-12_c.C | 10 ++++ > .../g++.dg/modules/tpl-friend-12_d.C | 8 +++ > .../g++.dg/modules/tpl-friend-12_e.C | 7 +++ > .../g++.dg/modules/tpl-friend-12_f.C | 8 +++ > .../g++.dg/modules/tpl-friend-13_a.C | 12 +++++ > .../g++.dg/modules/tpl-friend-13_b.C | 9 ++++ > .../g++.dg/modules/tpl-friend-13_c.C | 11 ++++ > .../g++.dg/modules/tpl-friend-13_d.C | 7 +++ > .../g++.dg/modules/tpl-friend-13_e.C | 14 +++++ > gcc/testsuite/g++.dg/modules/tpl-friend-9.C | 13 +++++ > 22 files changed, 299 insertions(+), 16 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-9.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index faa7a0052a5..67cc7d7bcec 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7417,6 +7417,7 @@ extern unsigned get_importing_module (tree, bool = false) ATTRIBUTE_PURE; > extern void set_instantiating_module (tree); > extern void set_defining_module (tree); > extern void maybe_key_decl (tree ctx, tree decl); > +extern void propagate_defining_module (tree decl, tree orig); > > extern void mangle_module (int m, bool include_partition); > extern void mangle_module_fini (); > @@ -7649,6 +7650,7 @@ extern bool template_guide_p (const_tree); > extern bool builtin_guide_p (const_tree); > extern void store_explicit_specifier (tree, tree); > extern tree lookup_explicit_specifier (tree); > +extern tree lookup_imported_hidden_friend (tree); > extern void walk_specializations (bool, > void (*)(bool, spec_entry *, > void *), > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index aa66da4829d..ba8689efe21 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -2276,30 +2276,34 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) > > if (modules_p () > && TREE_CODE (CP_DECL_CONTEXT (olddecl)) == NAMESPACE_DECL > - && TREE_CODE (olddecl) != NAMESPACE_DECL > - && !hiding) > + && TREE_CODE (olddecl) != NAMESPACE_DECL) > { > if (!module_may_redeclare (olddecl, newdecl)) > return error_mark_node; > > - tree not_tmpl = STRIP_TEMPLATE (olddecl); > - if (DECL_LANG_SPECIFIC (not_tmpl) > - && DECL_MODULE_ATTACH_P (not_tmpl) > - /* Typedefs are not entities and so are OK to be redeclared > - as exported: see [module.interface]/p6. */ > - && TREE_CODE (olddecl) != TYPE_DECL) > + if (!hiding) > { > - if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) > - && !DECL_MODULE_EXPORT_P (not_tmpl)) > + /* Hidden friends declarations just use exportingness of the > + old declaration; see CWG2588. */ > + tree not_tmpl = STRIP_TEMPLATE (olddecl); > + if (DECL_LANG_SPECIFIC (not_tmpl) > + && DECL_MODULE_ATTACH_P (not_tmpl) > + /* Typedefs are not entities and so are OK to be redeclared > + as exported: see [module.interface]/p6. */ > + && TREE_CODE (olddecl) != TYPE_DECL) > { > - auto_diagnostic_group d; > - error ("conflicting exporting for declaration %qD", newdecl); > - inform (olddecl_loc, > - "previously declared here without exporting"); > + if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) > + && !DECL_MODULE_EXPORT_P (not_tmpl)) > + { > + auto_diagnostic_group d; > + error ("conflicting exporting for declaration %qD", newdecl); > + inform (olddecl_loc, > + "previously declared here without exporting"); > + } > } > + else if (DECL_MODULE_EXPORT_P (newdecl)) > + DECL_MODULE_EXPORT_P (not_tmpl) = true; > } > - else if (DECL_MODULE_EXPORT_P (newdecl)) > - DECL_MODULE_EXPORT_P (not_tmpl) = true; > } > > /* We have committed to returning OLDDECL at this point. */ > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index e2d2910ae48..1a064e4ea79 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -2727,6 +2727,11 @@ vec<tree, va_heap, vl_embed> *post_load_decls; > typedef hash_map<tree, auto_vec<tree>> keyed_map_t; > static keyed_map_t *keyed_table; > > +/* Instantiations of temploid friends imported from another module > + need to be owned by the same module as their instantiating template. > + This maps these to the template that instantiated them. */ > +static hash_map<tree, tree> *imported_temploid_friends; > + > /********************************************************************/ > /* Tree streaming. The tree streaming is very specific to the tree > structures themselves. A tag indicates the kind of tree being > @@ -7997,6 +8002,11 @@ trees_out::decl_value (tree decl, depset *dep) > } > } > > + /* Write imported temploid friends so that importers can reconstruct > + this information on stream-in. */ > + tree* slot = imported_temploid_friends->get (decl); > + tree_node (slot ? *slot : NULL_TREE); > + > bool is_typedef = false; > if (!type && TREE_CODE (inner) == TYPE_DECL) > { > @@ -8303,6 +8313,12 @@ trees_in::decl_value () > } > } > > + if (tree owner = tree_node ()) > + { > + bool exists = imported_temploid_friends->put (decl, owner); > + gcc_assert (exists == !is_new); > + } > + > /* Regular typedefs will have a NULL TREE_TYPE at this point. */ > unsigned tdef_flags = 0; > bool is_typedef = false; > @@ -18930,6 +18946,12 @@ get_originating_module_decl (tree decl) > && DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl)) > decl = TYPE_NAME (DECL_CHAIN (decl)); > > + /* An imported temploid friend is attached to the same module the > + befriending class was. */ > + if (imported_temploid_friends) > + if (tree *slot = imported_temploid_friends->get (decl)) > + decl = *slot; > + > int use; > if (tree ti = node_template_info (decl, use)) > { > @@ -19238,6 +19260,34 @@ maybe_key_decl (tree ctx, tree decl) > vec.safe_push (decl); > } > > +/* DECL is an instantiated friend that should be attached to the same > + module that ORIG is. */ > + > +void > +propagate_defining_module (tree decl, tree orig) > +{ > + if (!modules_p ()) > + return; > + > + tree not_tmpl = STRIP_TEMPLATE (orig); > + if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_ATTACH_P (not_tmpl)) > + { > + tree inner = STRIP_TEMPLATE (decl); > + retrofit_lang_decl (inner); > + DECL_MODULE_ATTACH_P (inner) = true; > + } > + > + if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_IMPORT_P (not_tmpl)) > + { > + bool exists = imported_temploid_friends->put (decl, orig); > + > + /* We should only be called if lookup for an existing decl > + failed, in which case there shouldn't already be an entry > + in the map. */ > + gcc_assert (!exists); > + } > +} > + > /* Create the flat name string. It is simplest to have it handy. */ > > void > @@ -20451,6 +20501,8 @@ init_modules (cpp_reader *reader) > pending_table = new pending_map_t (EXPERIMENT (1, 400)); > entity_map = new entity_map_t (EXPERIMENT (1, 400)); > vec_safe_reserve (entity_ary, EXPERIMENT (1, 400)); > + imported_temploid_friends > + = new hash_map<tree,tree> (EXPERIMENT (1, 400)); > } > > #if CHECKING_P > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index 7af7f00e34c..dd6e7b6eaea 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -4453,6 +4453,48 @@ push_local_binding (tree id, tree decl, bool is_using) > add_decl_to_level (b, decl); > } > > +/* Lookup the FRIEND_TMPL within all module imports. Used to dedup > + instantiations of temploid hidden friends from imported modules. */ > + > +tree > +lookup_imported_hidden_friend (tree friend_tmpl) > +{ > + tree inner = DECL_TEMPLATE_RESULT (friend_tmpl); > + if (!DECL_LANG_SPECIFIC (inner) > + || !DECL_MODULE_IMPORT_P (inner)) > + return NULL_TREE; > + > + tree name = DECL_NAME (inner); > + tree *slot = find_namespace_slot (current_namespace, name); > + if (!slot || !*slot || TREE_CODE (*slot) != BINDING_VECTOR) > + return NULL_TREE; > + > + /* Look in the appropriate slot, as with check_module_override. */ > + binding_slot mslot; > + if (named_module_p ()) > + mslot = BINDING_VECTOR_CLUSTER (*slot, BINDING_SLOT_PARTITION > + / BINDING_VECTOR_SLOTS_PER_CLUSTER) > + .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER]; > + else > + mslot = BINDING_VECTOR_CLUSTER (*slot, 0).slots[BINDING_SLOT_GLOBAL]; > + gcc_assert (!mslot.is_lazy ()); > + > + tree ovl = mslot; > + if (!ovl) > + return NULL_TREE; > + > + /* We're only interested in declarations coming from the same module > + of the friend class we're attempting to instantiate. */ > + int m = get_originating_module (friend_tmpl); > + gcc_assert (m != 0); > + > + for (ovl_iterator iter (ovl); iter; ++iter) > + if (get_originating_module (*iter) == m) > + return *iter; > + > + return NULL_TREE; > +} > + > > /* true means unconditionally make a BLOCK for the next level pushed. */ > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index 3b2106dd3f6..e7e7f2fbc3b 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -11512,6 +11512,10 @@ tsubst_friend_function (tree decl, tree args) > new_friend_result_template_info = DECL_TEMPLATE_INFO (not_tmpl); > } > > + /* We need to propagate module attachment for the new friend from the > + owner of this template. */ > + propagate_defining_module (new_friend, decl); > + > /* Inside pushdecl_namespace_level, we will push into the > current namespace. However, the friend function should go > into the namespace of the template. */ > @@ -11715,6 +11719,12 @@ tsubst_friend_class (tree friend_tmpl, tree args) > tmpl = lookup_name (DECL_NAME (friend_tmpl), LOOK_where::CLASS_NAMESPACE, > LOOK_want::NORMAL | LOOK_want::HIDDEN_FRIEND); > > + if (!tmpl) > + /* If we didn't find by name lookup, the type may still exist but as a > + 'hidden' import; we should check for this too to avoid accidentally > + instantiating a duplicate. */ > + tmpl = lookup_imported_hidden_friend (friend_tmpl); > + > if (tmpl && DECL_CLASS_TEMPLATE_P (tmpl)) > { > /* The friend template has already been declared. Just > @@ -11723,6 +11733,11 @@ tsubst_friend_class (tree friend_tmpl, tree args) > of course. We only need the innermost template parameters > because that is all that redeclare_class_template will look > at. */ > + > + if (modules_p ()) > + /* Check that we can redeclare TMPL in the current context. */ > + module_may_redeclare (tmpl, friend_tmpl); > + > if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl)) > > TMPL_ARGS_DEPTH (args)) > { > @@ -11772,6 +11787,10 @@ tsubst_friend_class (tree friend_tmpl, tree args) > args, tf_warning_or_error); > } > > + /* We need to propagate the attachment of the original template to the > + newly instantiated template type. */ > + propagate_defining_module (tmpl, friend_tmpl); > + > /* Inject this template into the enclosing namspace scope. */ > tmpl = pushdecl_namespace_level (tmpl, /*hiding=*/true); > } > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C > new file mode 100644 > index 00000000000..7547326e554 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C > @@ -0,0 +1,15 @@ > +// PR c++/105320 > +// { dg-additional-options "-fmodules-ts -Wno-global-module" } > +// { dg-module-cmi test_support } > + > +module; > +template<class> struct _Sp_atomic; > +template<class> struct shared_ptr { > + template<class> friend struct _Sp_atomic; > + using atomic_type = _Sp_atomic<int>; > +}; > +export module test_support; > +export > +template<class T> struct A { > + shared_ptr<T> data; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C > new file mode 100644 > index 00000000000..6b88ee4258b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C > @@ -0,0 +1,5 @@ > +// PR c++/105320 > +// { dg-additional-options "-fmodules-ts" } > + > +import test_support; > +A<int> a; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C > new file mode 100644 > index 00000000000..789bdeb64d5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C > @@ -0,0 +1,7 @@ > +// PR c++/105320 > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi user } > + > +export module user; > +import test_support; > +A<int> b; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C > new file mode 100644 > index 00000000000..f29eebd1a7f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C > @@ -0,0 +1,14 @@ > +// PR c++/114275 > +// { dg-additional-options "-fmodules-ts -Wno-global-module" } > +// { dg-module-cmi M } > + > +module; > + > +template <typename... _Elements> struct T; > + > +template <typename H> struct T<H> { > + template <typename...> friend struct T; > +}; > + > +export module M; > +export template <typename=void> void fun() { T<int> t; } > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C > new file mode 100644 > index 00000000000..5bf79998139 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C > @@ -0,0 +1,5 @@ > +// PR c++/114275 > +// { dg-additional-options "-fmodules-ts" } > + > +import M; > +int main() { fun(); } > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C > new file mode 100644 > index 00000000000..216dbf62c71 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C > @@ -0,0 +1,10 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi M:A } > + > +module M:A; > + > +template <typename T> struct A { > + template <typename U> friend struct B; > +private: > + int x = 42; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C > new file mode 100644 > index 00000000000..26e1c38b518 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C > @@ -0,0 +1,9 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi M:B } > + > +export module M:B; > +import :A; > + > +export template <typename U> struct B { > + int foo(A<U> a) { return a.x; } > +}; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C > new file mode 100644 > index 00000000000..e44c2819cfd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C > @@ -0,0 +1,10 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi M:C } > + > +export module M:C; > +import :A; > + > +template <typename T> struct B; > +export template <typename T, typename U> int bar(B<T> t, U u) { > + return t.foo(u); > +} > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C > new file mode 100644 > index 00000000000..9a575ad5046 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C > @@ -0,0 +1,8 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi M } > + > +export module M; > +export import :B; > +export import :C; > + > +export int go_in_module(); > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C > new file mode 100644 > index 00000000000..329d1e8b263 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C > @@ -0,0 +1,7 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +module M; > + > +int go_in_module() { > + return bar(B<int>{}, A<int>{}); > +} > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C > new file mode 100644 > index 00000000000..c9855663fbd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C > @@ -0,0 +1,8 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +import M; > + > +int main() { > + B<double> b{}; > + go_in_module(); > +} > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C > new file mode 100644 > index 00000000000..a044485248f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C > @@ -0,0 +1,12 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi M } > + > +export module M; > +export template <typename> struct A { > + template <typename> friend struct B; > +}; > + > +export template <typename> struct C { > + friend void f(); > + template <typename> friend void g(); > +}; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C > new file mode 100644 > index 00000000000..72dc8611e39 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C > @@ -0,0 +1,9 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +import M; > +A<int> a; > +template <typename> struct B {}; // { dg-error "conflicts with import" } > + > +C<int> c; > +void f() {} // { dg-error "conflicts with import" } > +template <typename> void g() {} // { dg-error "conflicts with import" } > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C > new file mode 100644 > index 00000000000..e1d2860bfe6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C > @@ -0,0 +1,11 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +import M; > +template <typename> struct B {}; // { dg-message "previously declared" } > +A<int> a; // { dg-message "required from here" } > + > +void f() {} // { dg-message "previously declared" } > +template <typename> void g(); // { dg-message "previously declared" } > +C<int> c; // { dg-message "required from here" } > + > +// { dg-error "conflicting declaration" "" { target *-*-* } 0 } > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C > new file mode 100644 > index 00000000000..44b0f441db2 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C > @@ -0,0 +1,7 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi O } > + > +export module O; > +export import M; > +A<int> a; > +C<int> c; > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C > new file mode 100644 > index 00000000000..cec899e426b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C > @@ -0,0 +1,14 @@ > +// { dg-additional-options "-fmodules-ts" } > + > +template <typename> struct B {}; // { dg-message "previous declaration" } > +void f() {} > +template <typename T> void g() {} > +import O; > +A<double> b; // { dg-message "required from here" } > +C<double> d; // { dg-message "required from here" } > + > +// specifically, B is defined in M, not O, despite the instantiation being in O > +// { dg-error "conflicting declaration \[^\n\r\]* B@M" "" { target *-*-* } 0 } > +// and similarly for f and g > +// { dg-error "conflicting declaration \[^\n\r\]* f@M" "" { target *-*-* } 0 } > +// { dg-error "conflicting declaration \[^\n\r\]* g@M" "" { target *-*-* } 0 } > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-9.C b/gcc/testsuite/g++.dg/modules/tpl-friend-9.C > new file mode 100644 > index 00000000000..c7216f0f8c1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-9.C > @@ -0,0 +1,13 @@ > +// PR c++/114275 > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi M } > + > +export module M; > + > +template<class> struct A { > + template<class> friend struct B; > + friend void C(); > +}; > +A<int> a; > +void C() {} > +template<class> struct B { }; > -- > 2.43.2 > >
On Wed, Apr 17, 2024 at 02:02:21PM -0400, Patrick Palka wrote: > On Mon, 15 Apr 2024, Nathaniel Shead wrote: > > > I'm not a huge fan of always streaming 'imported_temploid_friends' for > > all decls, but I don't think it adds much performance cost over adding a > > new flag to categorise decls that might be marked as such. > > IIUC this value is going to be almost always null which is encoded as a > single 0 byte, which should be fast to stream. But I wonder how much > larger <bits/stdc++.h> gets? Can we get away with streaming this value > only for TEMPLATE_DECLs? Yes, it should either just be a 0 byte or an additional backref somewhere, which will likely also be small. On my system it increases the size by 0.26%, from 31186800 bytes to 31268672. But I've just found that this patch has a bug anyway, in that it doesn't correctly dedup if the friend types are instantiated in two separate modules that are then both imported. I'll see what I need to do to fix this which may influence what we need to stream here. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > This patch fixes a number of issues with the handling of temploid friend > > declarations. > > > > The primary issue is that instantiations of friend declarations should > > attach the declaration to the same module as the befriending class, by > > [module.unit] p7.1 and [temp.friend] p2; this could be a different > > module from the current TU, and so needs special handling. > > Nice, your approach seems consistent with Nathan's comments in the past > about this issue: > > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603288.html > https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611215.htmlw > > > > > The other main issue here is that we can't assume that just because name > > lookup didn't find a definition for a hidden template class, it doesn't > > mean that it doesn't exist: it could be a non-exported entity that we've > > nevertheless streamed in from an imported module. We need to ensure > > that when instantiating friend classes that we return the same TYPE_DECL > > that we got from our imports, otherwise we will get later issues with > > 'duplicate_decls' (rightfully) complaining that they're different. > > > > This doesn't appear necessary for functions due to the existing name > > lookup handling already finding these hidden declarations. > > It does seem like a weird inconsistency that tsubst_friend_class needs > this workaround but not tsubst_friend_function. > > I wonder if we can relax duplicate_decls to treat an instantiated > template friend class as a redeclaration instead of complaining, > mirroring its behavior for functions, which in turn would let us get rid > of the name lookup in tsubst_friend_class and eliminate the need for > lookup_imported_hidden_friend? This may be too speculative/risky of > a refactoring at this stage though, and your approach has the nice > advantage of changing only modules code paths. Hm, that's a good idea. I've played around a little bit with trying this but I've gotten a little stuck, might try again later. It also feels a little silly to do a full instantiation of a (potentially) large type only to immediately throw it away when we could just look for it beforehand, but this does feel like a neater solution anyway. > In any case I hope we can fix this issue for GCC 14! LGTM overall. > > > > > PR c++/105320 > > PR c++/114275 > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (propagate_defining_module): Declare. > > (lookup_imported_hidden_friend): Declare. > > * decl.cc (duplicate_decls): Also check if hidden declarations > > can be redeclared in this module. > > * module.cc (imported_temploid_friends): New map. > > (init_modules): Initialize it. > > (trees_out::decl_value): Write it. > > (trees_in::decl_value): Read it. > > (get_originating_module_decl): Follow the owning decl for an > > imported temploid friend. > > (propagate_defining_module): New function. > > * name-lookup.cc (lookup_imported_hidden_friend): New function. > > * pt.cc (tsubst_friend_function): Propagate defining module for > > new friend functions. > > (tsubst_friend_class): Lookup imported hidden friends. Check > > for valid redeclaration. Propagate defining module for new > > friend classes. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/tpl-friend-10_a.C: New test. > > * g++.dg/modules/tpl-friend-10_b.C: New test. > > * g++.dg/modules/tpl-friend-10_c.C: New test. > > * g++.dg/modules/tpl-friend-11_a.C: New test. > > * g++.dg/modules/tpl-friend-11_b.C: New test. > > * g++.dg/modules/tpl-friend-12_a.C: New test. > > * g++.dg/modules/tpl-friend-12_b.C: New test. > > * g++.dg/modules/tpl-friend-12_c.C: New test. > > * g++.dg/modules/tpl-friend-12_d.C: New test. > > * g++.dg/modules/tpl-friend-12_e.C: New test. > > * g++.dg/modules/tpl-friend-12_f.C: New test. > > * g++.dg/modules/tpl-friend-13_a.C: New test. > > * g++.dg/modules/tpl-friend-13_b.C: New test. > > * g++.dg/modules/tpl-friend-13_c.C: New test. > > * g++.dg/modules/tpl-friend-13_d.C: New test. > > * g++.dg/modules/tpl-friend-13_e.C: New test. > > * g++.dg/modules/tpl-friend-9.C: New test. > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > --- > > gcc/cp/cp-tree.h | 2 + > > gcc/cp/decl.cc | 36 +++++++------ > > gcc/cp/module.cc | 52 +++++++++++++++++++ > > gcc/cp/name-lookup.cc | 42 +++++++++++++++ > > gcc/cp/pt.cc | 19 +++++++ > > .../g++.dg/modules/tpl-friend-10_a.C | 15 ++++++ > > .../g++.dg/modules/tpl-friend-10_b.C | 5 ++ > > .../g++.dg/modules/tpl-friend-10_c.C | 7 +++ > > .../g++.dg/modules/tpl-friend-11_a.C | 14 +++++ > > .../g++.dg/modules/tpl-friend-11_b.C | 5 ++ > > .../g++.dg/modules/tpl-friend-12_a.C | 10 ++++ > > .../g++.dg/modules/tpl-friend-12_b.C | 9 ++++ > > .../g++.dg/modules/tpl-friend-12_c.C | 10 ++++ > > .../g++.dg/modules/tpl-friend-12_d.C | 8 +++ > > .../g++.dg/modules/tpl-friend-12_e.C | 7 +++ > > .../g++.dg/modules/tpl-friend-12_f.C | 8 +++ > > .../g++.dg/modules/tpl-friend-13_a.C | 12 +++++ > > .../g++.dg/modules/tpl-friend-13_b.C | 9 ++++ > > .../g++.dg/modules/tpl-friend-13_c.C | 11 ++++ > > .../g++.dg/modules/tpl-friend-13_d.C | 7 +++ > > .../g++.dg/modules/tpl-friend-13_e.C | 14 +++++ > > gcc/testsuite/g++.dg/modules/tpl-friend-9.C | 13 +++++ > > 22 files changed, 299 insertions(+), 16 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C > > create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-9.C > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index faa7a0052a5..67cc7d7bcec 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -7417,6 +7417,7 @@ extern unsigned get_importing_module (tree, bool = false) ATTRIBUTE_PURE; > > extern void set_instantiating_module (tree); > > extern void set_defining_module (tree); > > extern void maybe_key_decl (tree ctx, tree decl); > > +extern void propagate_defining_module (tree decl, tree orig); > > > > extern void mangle_module (int m, bool include_partition); > > extern void mangle_module_fini (); > > @@ -7649,6 +7650,7 @@ extern bool template_guide_p (const_tree); > > extern bool builtin_guide_p (const_tree); > > extern void store_explicit_specifier (tree, tree); > > extern tree lookup_explicit_specifier (tree); > > +extern tree lookup_imported_hidden_friend (tree); > > extern void walk_specializations (bool, > > void (*)(bool, spec_entry *, > > void *), > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > > index aa66da4829d..ba8689efe21 100644 > > --- a/gcc/cp/decl.cc > > +++ b/gcc/cp/decl.cc > > @@ -2276,30 +2276,34 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) > > > > if (modules_p () > > && TREE_CODE (CP_DECL_CONTEXT (olddecl)) == NAMESPACE_DECL > > - && TREE_CODE (olddecl) != NAMESPACE_DECL > > - && !hiding) > > + && TREE_CODE (olddecl) != NAMESPACE_DECL) > > { > > if (!module_may_redeclare (olddecl, newdecl)) > > return error_mark_node; > > > > - tree not_tmpl = STRIP_TEMPLATE (olddecl); > > - if (DECL_LANG_SPECIFIC (not_tmpl) > > - && DECL_MODULE_ATTACH_P (not_tmpl) > > - /* Typedefs are not entities and so are OK to be redeclared > > - as exported: see [module.interface]/p6. */ > > - && TREE_CODE (olddecl) != TYPE_DECL) > > + if (!hiding) > > { > > - if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) > > - && !DECL_MODULE_EXPORT_P (not_tmpl)) > > + /* Hidden friends declarations just use exportingness of the > > + old declaration; see CWG2588. */ > > + tree not_tmpl = STRIP_TEMPLATE (olddecl); > > + if (DECL_LANG_SPECIFIC (not_tmpl) > > + && DECL_MODULE_ATTACH_P (not_tmpl) > > + /* Typedefs are not entities and so are OK to be redeclared > > + as exported: see [module.interface]/p6. */ > > + && TREE_CODE (olddecl) != TYPE_DECL) > > { > > - auto_diagnostic_group d; > > - error ("conflicting exporting for declaration %qD", newdecl); > > - inform (olddecl_loc, > > - "previously declared here without exporting"); > > + if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) > > + && !DECL_MODULE_EXPORT_P (not_tmpl)) > > + { > > + auto_diagnostic_group d; > > + error ("conflicting exporting for declaration %qD", newdecl); > > + inform (olddecl_loc, > > + "previously declared here without exporting"); > > + } > > } > > + else if (DECL_MODULE_EXPORT_P (newdecl)) > > + DECL_MODULE_EXPORT_P (not_tmpl) = true; > > } > > - else if (DECL_MODULE_EXPORT_P (newdecl)) > > - DECL_MODULE_EXPORT_P (not_tmpl) = true; > > } > > > > /* We have committed to returning OLDDECL at this point. */ > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > > index e2d2910ae48..1a064e4ea79 100644 > > --- a/gcc/cp/module.cc > > +++ b/gcc/cp/module.cc > > @@ -2727,6 +2727,11 @@ vec<tree, va_heap, vl_embed> *post_load_decls; > > typedef hash_map<tree, auto_vec<tree>> keyed_map_t; > > static keyed_map_t *keyed_table; > > > > +/* Instantiations of temploid friends imported from another module > > + need to be owned by the same module as their instantiating template. > > + This maps these to the template that instantiated them. */ > > +static hash_map<tree, tree> *imported_temploid_friends; > > + > > /********************************************************************/ > > /* Tree streaming. The tree streaming is very specific to the tree > > structures themselves. A tag indicates the kind of tree being > > @@ -7997,6 +8002,11 @@ trees_out::decl_value (tree decl, depset *dep) > > } > > } > > > > + /* Write imported temploid friends so that importers can reconstruct > > + this information on stream-in. */ > > + tree* slot = imported_temploid_friends->get (decl); > > + tree_node (slot ? *slot : NULL_TREE); > > + > > bool is_typedef = false; > > if (!type && TREE_CODE (inner) == TYPE_DECL) > > { > > @@ -8303,6 +8313,12 @@ trees_in::decl_value () > > } > > } > > > > + if (tree owner = tree_node ()) > > + { > > + bool exists = imported_temploid_friends->put (decl, owner); > > + gcc_assert (exists == !is_new); > > + } > > + > > /* Regular typedefs will have a NULL TREE_TYPE at this point. */ > > unsigned tdef_flags = 0; > > bool is_typedef = false; > > @@ -18930,6 +18946,12 @@ get_originating_module_decl (tree decl) > > && DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl)) > > decl = TYPE_NAME (DECL_CHAIN (decl)); > > > > + /* An imported temploid friend is attached to the same module the > > + befriending class was. */ > > + if (imported_temploid_friends) > > + if (tree *slot = imported_temploid_friends->get (decl)) > > + decl = *slot; > > + > > int use; > > if (tree ti = node_template_info (decl, use)) > > { > > @@ -19238,6 +19260,34 @@ maybe_key_decl (tree ctx, tree decl) > > vec.safe_push (decl); > > } > > > > +/* DECL is an instantiated friend that should be attached to the same > > + module that ORIG is. */ > > + > > +void > > +propagate_defining_module (tree decl, tree orig) > > +{ > > + if (!modules_p ()) > > + return; > > + > > + tree not_tmpl = STRIP_TEMPLATE (orig); > > + if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_ATTACH_P (not_tmpl)) > > + { > > + tree inner = STRIP_TEMPLATE (decl); > > + retrofit_lang_decl (inner); > > + DECL_MODULE_ATTACH_P (inner) = true; > > + } > > + > > + if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_IMPORT_P (not_tmpl)) > > + { > > + bool exists = imported_temploid_friends->put (decl, orig); > > + > > + /* We should only be called if lookup for an existing decl > > + failed, in which case there shouldn't already be an entry > > + in the map. */ > > + gcc_assert (!exists); > > + } > > +} > > + > > /* Create the flat name string. It is simplest to have it handy. */ > > > > void > > @@ -20451,6 +20501,8 @@ init_modules (cpp_reader *reader) > > pending_table = new pending_map_t (EXPERIMENT (1, 400)); > > entity_map = new entity_map_t (EXPERIMENT (1, 400)); > > vec_safe_reserve (entity_ary, EXPERIMENT (1, 400)); > > + imported_temploid_friends > > + = new hash_map<tree,tree> (EXPERIMENT (1, 400)); > > } > > > > #if CHECKING_P > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > > index 7af7f00e34c..dd6e7b6eaea 100644 > > --- a/gcc/cp/name-lookup.cc > > +++ b/gcc/cp/name-lookup.cc > > @@ -4453,6 +4453,48 @@ push_local_binding (tree id, tree decl, bool is_using) > > add_decl_to_level (b, decl); > > } > > > > +/* Lookup the FRIEND_TMPL within all module imports. Used to dedup > > + instantiations of temploid hidden friends from imported modules. */ > > + > > +tree > > +lookup_imported_hidden_friend (tree friend_tmpl) > > +{ > > + tree inner = DECL_TEMPLATE_RESULT (friend_tmpl); > > + if (!DECL_LANG_SPECIFIC (inner) > > + || !DECL_MODULE_IMPORT_P (inner)) > > + return NULL_TREE; > > + > > + tree name = DECL_NAME (inner); > > + tree *slot = find_namespace_slot (current_namespace, name); > > + if (!slot || !*slot || TREE_CODE (*slot) != BINDING_VECTOR) > > + return NULL_TREE; > > + > > + /* Look in the appropriate slot, as with check_module_override. */ > > + binding_slot mslot; > > + if (named_module_p ()) > > + mslot = BINDING_VECTOR_CLUSTER (*slot, BINDING_SLOT_PARTITION > > + / BINDING_VECTOR_SLOTS_PER_CLUSTER) > > + .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER]; > > + else > > + mslot = BINDING_VECTOR_CLUSTER (*slot, 0).slots[BINDING_SLOT_GLOBAL]; > > + gcc_assert (!mslot.is_lazy ()); > > + > > + tree ovl = mslot; > > + if (!ovl) > > + return NULL_TREE; > > + > > + /* We're only interested in declarations coming from the same module > > + of the friend class we're attempting to instantiate. */ > > + int m = get_originating_module (friend_tmpl); > > + gcc_assert (m != 0); > > + > > + for (ovl_iterator iter (ovl); iter; ++iter) > > + if (get_originating_module (*iter) == m) > > + return *iter; > > + > > + return NULL_TREE; > > +} > > + > > > > /* true means unconditionally make a BLOCK for the next level pushed. */ > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index 3b2106dd3f6..e7e7f2fbc3b 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -11512,6 +11512,10 @@ tsubst_friend_function (tree decl, tree args) > > new_friend_result_template_info = DECL_TEMPLATE_INFO (not_tmpl); > > } > > > > + /* We need to propagate module attachment for the new friend from the > > + owner of this template. */ > > + propagate_defining_module (new_friend, decl); > > + > > /* Inside pushdecl_namespace_level, we will push into the > > current namespace. However, the friend function should go > > into the namespace of the template. */ > > @@ -11715,6 +11719,12 @@ tsubst_friend_class (tree friend_tmpl, tree args) > > tmpl = lookup_name (DECL_NAME (friend_tmpl), LOOK_where::CLASS_NAMESPACE, > > LOOK_want::NORMAL | LOOK_want::HIDDEN_FRIEND); > > > > + if (!tmpl) > > + /* If we didn't find by name lookup, the type may still exist but as a > > + 'hidden' import; we should check for this too to avoid accidentally > > + instantiating a duplicate. */ > > + tmpl = lookup_imported_hidden_friend (friend_tmpl); > > + > > if (tmpl && DECL_CLASS_TEMPLATE_P (tmpl)) > > { > > /* The friend template has already been declared. Just > > @@ -11723,6 +11733,11 @@ tsubst_friend_class (tree friend_tmpl, tree args) > > of course. We only need the innermost template parameters > > because that is all that redeclare_class_template will look > > at. */ > > + > > + if (modules_p ()) > > + /* Check that we can redeclare TMPL in the current context. */ > > + module_may_redeclare (tmpl, friend_tmpl); > > + > > if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl)) > > > TMPL_ARGS_DEPTH (args)) > > { > > @@ -11772,6 +11787,10 @@ tsubst_friend_class (tree friend_tmpl, tree args) > > args, tf_warning_or_error); > > } > > > > + /* We need to propagate the attachment of the original template to the > > + newly instantiated template type. */ > > + propagate_defining_module (tmpl, friend_tmpl); > > + > > /* Inject this template into the enclosing namspace scope. */ > > tmpl = pushdecl_namespace_level (tmpl, /*hiding=*/true); > > } > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C > > new file mode 100644 > > index 00000000000..7547326e554 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C > > @@ -0,0 +1,15 @@ > > +// PR c++/105320 > > +// { dg-additional-options "-fmodules-ts -Wno-global-module" } > > +// { dg-module-cmi test_support } > > + > > +module; > > +template<class> struct _Sp_atomic; > > +template<class> struct shared_ptr { > > + template<class> friend struct _Sp_atomic; > > + using atomic_type = _Sp_atomic<int>; > > +}; > > +export module test_support; > > +export > > +template<class T> struct A { > > + shared_ptr<T> data; > > +}; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C > > new file mode 100644 > > index 00000000000..6b88ee4258b > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C > > @@ -0,0 +1,5 @@ > > +// PR c++/105320 > > +// { dg-additional-options "-fmodules-ts" } > > + > > +import test_support; > > +A<int> a; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C > > new file mode 100644 > > index 00000000000..789bdeb64d5 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C > > @@ -0,0 +1,7 @@ > > +// PR c++/105320 > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi user } > > + > > +export module user; > > +import test_support; > > +A<int> b; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C > > new file mode 100644 > > index 00000000000..f29eebd1a7f > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C > > @@ -0,0 +1,14 @@ > > +// PR c++/114275 > > +// { dg-additional-options "-fmodules-ts -Wno-global-module" } > > +// { dg-module-cmi M } > > + > > +module; > > + > > +template <typename... _Elements> struct T; > > + > > +template <typename H> struct T<H> { > > + template <typename...> friend struct T; > > +}; > > + > > +export module M; > > +export template <typename=void> void fun() { T<int> t; } > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C > > new file mode 100644 > > index 00000000000..5bf79998139 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C > > @@ -0,0 +1,5 @@ > > +// PR c++/114275 > > +// { dg-additional-options "-fmodules-ts" } > > + > > +import M; > > +int main() { fun(); } > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C > > new file mode 100644 > > index 00000000000..216dbf62c71 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C > > @@ -0,0 +1,10 @@ > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi M:A } > > + > > +module M:A; > > + > > +template <typename T> struct A { > > + template <typename U> friend struct B; > > +private: > > + int x = 42; > > +}; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C > > new file mode 100644 > > index 00000000000..26e1c38b518 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C > > @@ -0,0 +1,9 @@ > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi M:B } > > + > > +export module M:B; > > +import :A; > > + > > +export template <typename U> struct B { > > + int foo(A<U> a) { return a.x; } > > +}; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C > > new file mode 100644 > > index 00000000000..e44c2819cfd > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C > > @@ -0,0 +1,10 @@ > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi M:C } > > + > > +export module M:C; > > +import :A; > > + > > +template <typename T> struct B; > > +export template <typename T, typename U> int bar(B<T> t, U u) { > > + return t.foo(u); > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C > > new file mode 100644 > > index 00000000000..9a575ad5046 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C > > @@ -0,0 +1,8 @@ > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi M } > > + > > +export module M; > > +export import :B; > > +export import :C; > > + > > +export int go_in_module(); > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C > > new file mode 100644 > > index 00000000000..329d1e8b263 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C > > @@ -0,0 +1,7 @@ > > +// { dg-additional-options "-fmodules-ts" } > > + > > +module M; > > + > > +int go_in_module() { > > + return bar(B<int>{}, A<int>{}); > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C > > new file mode 100644 > > index 00000000000..c9855663fbd > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C > > @@ -0,0 +1,8 @@ > > +// { dg-additional-options "-fmodules-ts" } > > + > > +import M; > > + > > +int main() { > > + B<double> b{}; > > + go_in_module(); > > +} > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C > > new file mode 100644 > > index 00000000000..a044485248f > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C > > @@ -0,0 +1,12 @@ > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi M } > > + > > +export module M; > > +export template <typename> struct A { > > + template <typename> friend struct B; > > +}; > > + > > +export template <typename> struct C { > > + friend void f(); > > + template <typename> friend void g(); > > +}; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C > > new file mode 100644 > > index 00000000000..72dc8611e39 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C > > @@ -0,0 +1,9 @@ > > +// { dg-additional-options "-fmodules-ts" } > > + > > +import M; > > +A<int> a; > > +template <typename> struct B {}; // { dg-error "conflicts with import" } > > + > > +C<int> c; > > +void f() {} // { dg-error "conflicts with import" } > > +template <typename> void g() {} // { dg-error "conflicts with import" } > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C > > new file mode 100644 > > index 00000000000..e1d2860bfe6 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C > > @@ -0,0 +1,11 @@ > > +// { dg-additional-options "-fmodules-ts" } > > + > > +import M; > > +template <typename> struct B {}; // { dg-message "previously declared" } > > +A<int> a; // { dg-message "required from here" } > > + > > +void f() {} // { dg-message "previously declared" } > > +template <typename> void g(); // { dg-message "previously declared" } > > +C<int> c; // { dg-message "required from here" } > > + > > +// { dg-error "conflicting declaration" "" { target *-*-* } 0 } > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C > > new file mode 100644 > > index 00000000000..44b0f441db2 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C > > @@ -0,0 +1,7 @@ > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi O } > > + > > +export module O; > > +export import M; > > +A<int> a; > > +C<int> c; > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C > > new file mode 100644 > > index 00000000000..cec899e426b > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C > > @@ -0,0 +1,14 @@ > > +// { dg-additional-options "-fmodules-ts" } > > + > > +template <typename> struct B {}; // { dg-message "previous declaration" } > > +void f() {} > > +template <typename T> void g() {} > > +import O; > > +A<double> b; // { dg-message "required from here" } > > +C<double> d; // { dg-message "required from here" } > > + > > +// specifically, B is defined in M, not O, despite the instantiation being in O > > +// { dg-error "conflicting declaration \[^\n\r\]* B@M" "" { target *-*-* } 0 } > > +// and similarly for f and g > > +// { dg-error "conflicting declaration \[^\n\r\]* f@M" "" { target *-*-* } 0 } > > +// { dg-error "conflicting declaration \[^\n\r\]* g@M" "" { target *-*-* } 0 } > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-9.C b/gcc/testsuite/g++.dg/modules/tpl-friend-9.C > > new file mode 100644 > > index 00000000000..c7216f0f8c1 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-9.C > > @@ -0,0 +1,13 @@ > > +// PR c++/114275 > > +// { dg-additional-options "-fmodules-ts" } > > +// { dg-module-cmi M } > > + > > +export module M; > > + > > +template<class> struct A { > > + template<class> friend struct B; > > + friend void C(); > > +}; > > +A<int> a; > > +void C() {} > > +template<class> struct B { }; > > -- > > 2.43.2 > > > > >
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index faa7a0052a5..67cc7d7bcec 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7417,6 +7417,7 @@ extern unsigned get_importing_module (tree, bool = false) ATTRIBUTE_PURE; extern void set_instantiating_module (tree); extern void set_defining_module (tree); extern void maybe_key_decl (tree ctx, tree decl); +extern void propagate_defining_module (tree decl, tree orig); extern void mangle_module (int m, bool include_partition); extern void mangle_module_fini (); @@ -7649,6 +7650,7 @@ extern bool template_guide_p (const_tree); extern bool builtin_guide_p (const_tree); extern void store_explicit_specifier (tree, tree); extern tree lookup_explicit_specifier (tree); +extern tree lookup_imported_hidden_friend (tree); extern void walk_specializations (bool, void (*)(bool, spec_entry *, void *), diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index aa66da4829d..ba8689efe21 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -2276,30 +2276,34 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) if (modules_p () && TREE_CODE (CP_DECL_CONTEXT (olddecl)) == NAMESPACE_DECL - && TREE_CODE (olddecl) != NAMESPACE_DECL - && !hiding) + && TREE_CODE (olddecl) != NAMESPACE_DECL) { if (!module_may_redeclare (olddecl, newdecl)) return error_mark_node; - tree not_tmpl = STRIP_TEMPLATE (olddecl); - if (DECL_LANG_SPECIFIC (not_tmpl) - && DECL_MODULE_ATTACH_P (not_tmpl) - /* Typedefs are not entities and so are OK to be redeclared - as exported: see [module.interface]/p6. */ - && TREE_CODE (olddecl) != TYPE_DECL) + if (!hiding) { - if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) - && !DECL_MODULE_EXPORT_P (not_tmpl)) + /* Hidden friends declarations just use exportingness of the + old declaration; see CWG2588. */ + tree not_tmpl = STRIP_TEMPLATE (olddecl); + if (DECL_LANG_SPECIFIC (not_tmpl) + && DECL_MODULE_ATTACH_P (not_tmpl) + /* Typedefs are not entities and so are OK to be redeclared + as exported: see [module.interface]/p6. */ + && TREE_CODE (olddecl) != TYPE_DECL) { - auto_diagnostic_group d; - error ("conflicting exporting for declaration %qD", newdecl); - inform (olddecl_loc, - "previously declared here without exporting"); + if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (newdecl)) + && !DECL_MODULE_EXPORT_P (not_tmpl)) + { + auto_diagnostic_group d; + error ("conflicting exporting for declaration %qD", newdecl); + inform (olddecl_loc, + "previously declared here without exporting"); + } } + else if (DECL_MODULE_EXPORT_P (newdecl)) + DECL_MODULE_EXPORT_P (not_tmpl) = true; } - else if (DECL_MODULE_EXPORT_P (newdecl)) - DECL_MODULE_EXPORT_P (not_tmpl) = true; } /* We have committed to returning OLDDECL at this point. */ diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index e2d2910ae48..1a064e4ea79 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -2727,6 +2727,11 @@ vec<tree, va_heap, vl_embed> *post_load_decls; typedef hash_map<tree, auto_vec<tree>> keyed_map_t; static keyed_map_t *keyed_table; +/* Instantiations of temploid friends imported from another module + need to be owned by the same module as their instantiating template. + This maps these to the template that instantiated them. */ +static hash_map<tree, tree> *imported_temploid_friends; + /********************************************************************/ /* Tree streaming. The tree streaming is very specific to the tree structures themselves. A tag indicates the kind of tree being @@ -7997,6 +8002,11 @@ trees_out::decl_value (tree decl, depset *dep) } } + /* Write imported temploid friends so that importers can reconstruct + this information on stream-in. */ + tree* slot = imported_temploid_friends->get (decl); + tree_node (slot ? *slot : NULL_TREE); + bool is_typedef = false; if (!type && TREE_CODE (inner) == TYPE_DECL) { @@ -8303,6 +8313,12 @@ trees_in::decl_value () } } + if (tree owner = tree_node ()) + { + bool exists = imported_temploid_friends->put (decl, owner); + gcc_assert (exists == !is_new); + } + /* Regular typedefs will have a NULL TREE_TYPE at this point. */ unsigned tdef_flags = 0; bool is_typedef = false; @@ -18930,6 +18946,12 @@ get_originating_module_decl (tree decl) && DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl)) decl = TYPE_NAME (DECL_CHAIN (decl)); + /* An imported temploid friend is attached to the same module the + befriending class was. */ + if (imported_temploid_friends) + if (tree *slot = imported_temploid_friends->get (decl)) + decl = *slot; + int use; if (tree ti = node_template_info (decl, use)) { @@ -19238,6 +19260,34 @@ maybe_key_decl (tree ctx, tree decl) vec.safe_push (decl); } +/* DECL is an instantiated friend that should be attached to the same + module that ORIG is. */ + +void +propagate_defining_module (tree decl, tree orig) +{ + if (!modules_p ()) + return; + + tree not_tmpl = STRIP_TEMPLATE (orig); + if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_ATTACH_P (not_tmpl)) + { + tree inner = STRIP_TEMPLATE (decl); + retrofit_lang_decl (inner); + DECL_MODULE_ATTACH_P (inner) = true; + } + + if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_IMPORT_P (not_tmpl)) + { + bool exists = imported_temploid_friends->put (decl, orig); + + /* We should only be called if lookup for an existing decl + failed, in which case there shouldn't already be an entry + in the map. */ + gcc_assert (!exists); + } +} + /* Create the flat name string. It is simplest to have it handy. */ void @@ -20451,6 +20501,8 @@ init_modules (cpp_reader *reader) pending_table = new pending_map_t (EXPERIMENT (1, 400)); entity_map = new entity_map_t (EXPERIMENT (1, 400)); vec_safe_reserve (entity_ary, EXPERIMENT (1, 400)); + imported_temploid_friends + = new hash_map<tree,tree> (EXPERIMENT (1, 400)); } #if CHECKING_P diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index 7af7f00e34c..dd6e7b6eaea 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -4453,6 +4453,48 @@ push_local_binding (tree id, tree decl, bool is_using) add_decl_to_level (b, decl); } +/* Lookup the FRIEND_TMPL within all module imports. Used to dedup + instantiations of temploid hidden friends from imported modules. */ + +tree +lookup_imported_hidden_friend (tree friend_tmpl) +{ + tree inner = DECL_TEMPLATE_RESULT (friend_tmpl); + if (!DECL_LANG_SPECIFIC (inner) + || !DECL_MODULE_IMPORT_P (inner)) + return NULL_TREE; + + tree name = DECL_NAME (inner); + tree *slot = find_namespace_slot (current_namespace, name); + if (!slot || !*slot || TREE_CODE (*slot) != BINDING_VECTOR) + return NULL_TREE; + + /* Look in the appropriate slot, as with check_module_override. */ + binding_slot mslot; + if (named_module_p ()) + mslot = BINDING_VECTOR_CLUSTER (*slot, BINDING_SLOT_PARTITION + / BINDING_VECTOR_SLOTS_PER_CLUSTER) + .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER]; + else + mslot = BINDING_VECTOR_CLUSTER (*slot, 0).slots[BINDING_SLOT_GLOBAL]; + gcc_assert (!mslot.is_lazy ()); + + tree ovl = mslot; + if (!ovl) + return NULL_TREE; + + /* We're only interested in declarations coming from the same module + of the friend class we're attempting to instantiate. */ + int m = get_originating_module (friend_tmpl); + gcc_assert (m != 0); + + for (ovl_iterator iter (ovl); iter; ++iter) + if (get_originating_module (*iter) == m) + return *iter; + + return NULL_TREE; +} + /* true means unconditionally make a BLOCK for the next level pushed. */ diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 3b2106dd3f6..e7e7f2fbc3b 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -11512,6 +11512,10 @@ tsubst_friend_function (tree decl, tree args) new_friend_result_template_info = DECL_TEMPLATE_INFO (not_tmpl); } + /* We need to propagate module attachment for the new friend from the + owner of this template. */ + propagate_defining_module (new_friend, decl); + /* Inside pushdecl_namespace_level, we will push into the current namespace. However, the friend function should go into the namespace of the template. */ @@ -11715,6 +11719,12 @@ tsubst_friend_class (tree friend_tmpl, tree args) tmpl = lookup_name (DECL_NAME (friend_tmpl), LOOK_where::CLASS_NAMESPACE, LOOK_want::NORMAL | LOOK_want::HIDDEN_FRIEND); + if (!tmpl) + /* If we didn't find by name lookup, the type may still exist but as a + 'hidden' import; we should check for this too to avoid accidentally + instantiating a duplicate. */ + tmpl = lookup_imported_hidden_friend (friend_tmpl); + if (tmpl && DECL_CLASS_TEMPLATE_P (tmpl)) { /* The friend template has already been declared. Just @@ -11723,6 +11733,11 @@ tsubst_friend_class (tree friend_tmpl, tree args) of course. We only need the innermost template parameters because that is all that redeclare_class_template will look at. */ + + if (modules_p ()) + /* Check that we can redeclare TMPL in the current context. */ + module_may_redeclare (tmpl, friend_tmpl); + if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl)) > TMPL_ARGS_DEPTH (args)) { @@ -11772,6 +11787,10 @@ tsubst_friend_class (tree friend_tmpl, tree args) args, tf_warning_or_error); } + /* We need to propagate the attachment of the original template to the + newly instantiated template type. */ + propagate_defining_module (tmpl, friend_tmpl); + /* Inject this template into the enclosing namspace scope. */ tmpl = pushdecl_namespace_level (tmpl, /*hiding=*/true); } diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C new file mode 100644 index 00000000000..7547326e554 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C @@ -0,0 +1,15 @@ +// PR c++/105320 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi test_support } + +module; +template<class> struct _Sp_atomic; +template<class> struct shared_ptr { + template<class> friend struct _Sp_atomic; + using atomic_type = _Sp_atomic<int>; +}; +export module test_support; +export +template<class T> struct A { + shared_ptr<T> data; +}; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C new file mode 100644 index 00000000000..6b88ee4258b --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C @@ -0,0 +1,5 @@ +// PR c++/105320 +// { dg-additional-options "-fmodules-ts" } + +import test_support; +A<int> a; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C new file mode 100644 index 00000000000..789bdeb64d5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C @@ -0,0 +1,7 @@ +// PR c++/105320 +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi user } + +export module user; +import test_support; +A<int> b; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C new file mode 100644 index 00000000000..f29eebd1a7f --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C @@ -0,0 +1,14 @@ +// PR c++/114275 +// { dg-additional-options "-fmodules-ts -Wno-global-module" } +// { dg-module-cmi M } + +module; + +template <typename... _Elements> struct T; + +template <typename H> struct T<H> { + template <typename...> friend struct T; +}; + +export module M; +export template <typename=void> void fun() { T<int> t; } diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C new file mode 100644 index 00000000000..5bf79998139 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C @@ -0,0 +1,5 @@ +// PR c++/114275 +// { dg-additional-options "-fmodules-ts" } + +import M; +int main() { fun(); } diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C new file mode 100644 index 00000000000..216dbf62c71 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C @@ -0,0 +1,10 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M:A } + +module M:A; + +template <typename T> struct A { + template <typename U> friend struct B; +private: + int x = 42; +}; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C new file mode 100644 index 00000000000..26e1c38b518 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C @@ -0,0 +1,9 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M:B } + +export module M:B; +import :A; + +export template <typename U> struct B { + int foo(A<U> a) { return a.x; } +}; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C new file mode 100644 index 00000000000..e44c2819cfd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C @@ -0,0 +1,10 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M:C } + +export module M:C; +import :A; + +template <typename T> struct B; +export template <typename T, typename U> int bar(B<T> t, U u) { + return t.foo(u); +} diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C new file mode 100644 index 00000000000..9a575ad5046 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C @@ -0,0 +1,8 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M } + +export module M; +export import :B; +export import :C; + +export int go_in_module(); diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C new file mode 100644 index 00000000000..329d1e8b263 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C @@ -0,0 +1,7 @@ +// { dg-additional-options "-fmodules-ts" } + +module M; + +int go_in_module() { + return bar(B<int>{}, A<int>{}); +} diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C b/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C new file mode 100644 index 00000000000..c9855663fbd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C @@ -0,0 +1,8 @@ +// { dg-additional-options "-fmodules-ts" } + +import M; + +int main() { + B<double> b{}; + go_in_module(); +} diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C new file mode 100644 index 00000000000..a044485248f --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C @@ -0,0 +1,12 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M } + +export module M; +export template <typename> struct A { + template <typename> friend struct B; +}; + +export template <typename> struct C { + friend void f(); + template <typename> friend void g(); +}; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C new file mode 100644 index 00000000000..72dc8611e39 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C @@ -0,0 +1,9 @@ +// { dg-additional-options "-fmodules-ts" } + +import M; +A<int> a; +template <typename> struct B {}; // { dg-error "conflicts with import" } + +C<int> c; +void f() {} // { dg-error "conflicts with import" } +template <typename> void g() {} // { dg-error "conflicts with import" } diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C new file mode 100644 index 00000000000..e1d2860bfe6 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C @@ -0,0 +1,11 @@ +// { dg-additional-options "-fmodules-ts" } + +import M; +template <typename> struct B {}; // { dg-message "previously declared" } +A<int> a; // { dg-message "required from here" } + +void f() {} // { dg-message "previously declared" } +template <typename> void g(); // { dg-message "previously declared" } +C<int> c; // { dg-message "required from here" } + +// { dg-error "conflicting declaration" "" { target *-*-* } 0 } diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C new file mode 100644 index 00000000000..44b0f441db2 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C @@ -0,0 +1,7 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi O } + +export module O; +export import M; +A<int> a; +C<int> c; diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C new file mode 100644 index 00000000000..cec899e426b --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C @@ -0,0 +1,14 @@ +// { dg-additional-options "-fmodules-ts" } + +template <typename> struct B {}; // { dg-message "previous declaration" } +void f() {} +template <typename T> void g() {} +import O; +A<double> b; // { dg-message "required from here" } +C<double> d; // { dg-message "required from here" } + +// specifically, B is defined in M, not O, despite the instantiation being in O +// { dg-error "conflicting declaration \[^\n\r\]* B@M" "" { target *-*-* } 0 } +// and similarly for f and g +// { dg-error "conflicting declaration \[^\n\r\]* f@M" "" { target *-*-* } 0 } +// { dg-error "conflicting declaration \[^\n\r\]* g@M" "" { target *-*-* } 0 } diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-9.C b/gcc/testsuite/g++.dg/modules/tpl-friend-9.C new file mode 100644 index 00000000000..c7216f0f8c1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-9.C @@ -0,0 +1,13 @@ +// PR c++/114275 +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi M } + +export module M; + +template<class> struct A { + template<class> friend struct B; + friend void C(); +}; +A<int> a; +void C() {} +template<class> struct B { };
I'm not a huge fan of always streaming 'imported_temploid_friends' for all decls, but I don't think it adds much performance cost over adding a new flag to categorise decls that might be marked as such. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- This patch fixes a number of issues with the handling of temploid friend declarations. The primary issue is that instantiations of friend declarations should attach the declaration to the same module as the befriending class, by [module.unit] p7.1 and [temp.friend] p2; this could be a different module from the current TU, and so needs special handling. The other main issue here is that we can't assume that just because name lookup didn't find a definition for a hidden template class, it doesn't mean that it doesn't exist: it could be a non-exported entity that we've nevertheless streamed in from an imported module. We need to ensure that when instantiating friend classes that we return the same TYPE_DECL that we got from our imports, otherwise we will get later issues with 'duplicate_decls' (rightfully) complaining that they're different. This doesn't appear necessary for functions due to the existing name lookup handling already finding these hidden declarations. PR c++/105320 PR c++/114275 gcc/cp/ChangeLog: * cp-tree.h (propagate_defining_module): Declare. (lookup_imported_hidden_friend): Declare. * decl.cc (duplicate_decls): Also check if hidden declarations can be redeclared in this module. * module.cc (imported_temploid_friends): New map. (init_modules): Initialize it. (trees_out::decl_value): Write it. (trees_in::decl_value): Read it. (get_originating_module_decl): Follow the owning decl for an imported temploid friend. (propagate_defining_module): New function. * name-lookup.cc (lookup_imported_hidden_friend): New function. * pt.cc (tsubst_friend_function): Propagate defining module for new friend functions. (tsubst_friend_class): Lookup imported hidden friends. Check for valid redeclaration. Propagate defining module for new friend classes. gcc/testsuite/ChangeLog: * g++.dg/modules/tpl-friend-10_a.C: New test. * g++.dg/modules/tpl-friend-10_b.C: New test. * g++.dg/modules/tpl-friend-10_c.C: New test. * g++.dg/modules/tpl-friend-11_a.C: New test. * g++.dg/modules/tpl-friend-11_b.C: New test. * g++.dg/modules/tpl-friend-12_a.C: New test. * g++.dg/modules/tpl-friend-12_b.C: New test. * g++.dg/modules/tpl-friend-12_c.C: New test. * g++.dg/modules/tpl-friend-12_d.C: New test. * g++.dg/modules/tpl-friend-12_e.C: New test. * g++.dg/modules/tpl-friend-12_f.C: New test. * g++.dg/modules/tpl-friend-13_a.C: New test. * g++.dg/modules/tpl-friend-13_b.C: New test. * g++.dg/modules/tpl-friend-13_c.C: New test. * g++.dg/modules/tpl-friend-13_d.C: New test. * g++.dg/modules/tpl-friend-13_e.C: New test. * g++.dg/modules/tpl-friend-9.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/cp-tree.h | 2 + gcc/cp/decl.cc | 36 +++++++------ gcc/cp/module.cc | 52 +++++++++++++++++++ gcc/cp/name-lookup.cc | 42 +++++++++++++++ gcc/cp/pt.cc | 19 +++++++ .../g++.dg/modules/tpl-friend-10_a.C | 15 ++++++ .../g++.dg/modules/tpl-friend-10_b.C | 5 ++ .../g++.dg/modules/tpl-friend-10_c.C | 7 +++ .../g++.dg/modules/tpl-friend-11_a.C | 14 +++++ .../g++.dg/modules/tpl-friend-11_b.C | 5 ++ .../g++.dg/modules/tpl-friend-12_a.C | 10 ++++ .../g++.dg/modules/tpl-friend-12_b.C | 9 ++++ .../g++.dg/modules/tpl-friend-12_c.C | 10 ++++ .../g++.dg/modules/tpl-friend-12_d.C | 8 +++ .../g++.dg/modules/tpl-friend-12_e.C | 7 +++ .../g++.dg/modules/tpl-friend-12_f.C | 8 +++ .../g++.dg/modules/tpl-friend-13_a.C | 12 +++++ .../g++.dg/modules/tpl-friend-13_b.C | 9 ++++ .../g++.dg/modules/tpl-friend-13_c.C | 11 ++++ .../g++.dg/modules/tpl-friend-13_d.C | 7 +++ .../g++.dg/modules/tpl-friend-13_e.C | 14 +++++ gcc/testsuite/g++.dg/modules/tpl-friend-9.C | 13 +++++ 22 files changed, 299 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_a.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_b.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-10_c.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_a.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-11_b.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_a.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_b.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_c.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_d.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_e.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-12_f.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_d.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-9.C