Re: [PATCH][14 backport] c++: Fix instantiation of imported temploid friends [PR114275]

[Patrick Palka <ppalka@redhat.com>]

On Mon, 13 May 2024, Nathaniel Shead wrote:

> > > @@ -11751,9 +11767,16 @@ tsubst_friend_class (tree friend_tmpl, tree args)
> > >         if (tmpl != error_mark_node)
> > >   	{
> > >   	  /* The new TMPL is not an instantiation of anything, so we
> > > -	     forget its origins.  We don't reset CLASSTYPE_TI_TEMPLATE
> > > +	     forget its origins.  It is also not a specialization of
> > > +	     anything.  We don't reset CLASSTYPE_TI_TEMPLATE
> > >   	     for the new type because that is supposed to be the
> > >   	     corresponding template decl, i.e., TMPL.  */
> > > +	  spec_entry elt;
> > > +	  elt.tmpl = friend_tmpl;
> > > +	  elt.args = CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl));
> > > +	  elt.spec = TREE_TYPE (tmpl);
> > > +	  type_specializations->remove_elt (&elt);
> > 
> > For GCC 14.2 let's guard this with if (modules_p ()); for GCC 15 it can be
> > unconditional.  OK.
> > 
> > Jason
> > 
> 
> I'm looking to backport this patch to GCC 14 now that it's been on trunk
> some time.  Here's the patch I'm aiming to add (squashed with the
> changes from r15-220-gec2365e07537e8) after cherrypicking the
> prerequisite commit r15-58-g2faf040335f9b4; is this OK?
> 
> Or should I keep it as two separate commits to make the cherrypicking
> more obvious? Not entirely sure on the etiquette around this.

Since the first patch "only" causes sporadic testsuite failures (and
doesn't e.g. break bootstrap or anything serious like that), I reckon
it'd be fine to keep them as separate commits?  Not sure either.

> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu on top of the
> releases/gcc-14 branch.
> 
> -- >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 class template, that it
> doesn't exist at all: it could be a non-exported entity that we've
> nevertheless streamed in from an imported module.  We need to ensure
> that when instantiating template friend classes that we return the same
> TEMPLATE_DECL that we got from our imports, otherwise we will get later
> issues with 'duplicate_decls' (rightfully) complaining that they're
> different when trying to merge.
> 
> This doesn't appear necessary for function templates 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.
> 	(remove_defining_module): Declare.
> 	(lookup_imported_hidden_friend): Declare.
> 	* decl.cc (duplicate_decls): Also check if hidden decls can be
> 	redeclared in this module. Call remove_defining_module on
> 	to-be-freed newdecl.
> 	* module.cc (imported_temploid_friends): New.
> 	(init_modules): Initialize it.
> 	(trees_out::decl_value): Write it; don't consider imported
> 	temploid friends as attached to a module.
> 	(trees_in::decl_value): Read it for non-discarded decls.
> 	(get_originating_module_decl): Follow the owning decl for an
> 	imported temploid friend.
> 	(propagate_defining_module): New.
> 	(remove_defining_module): New.
> 	* name-lookup.cc (get_mergeable_namespace_binding): New.
> 	(lookup_imported_hidden_friend): New.
> 	* pt.cc (tsubst_friend_function): Propagate defining module for
> 	new friend functions.
> 	(tsubst_friend_class): Lookup imported hidden friends.  Check
> 	for valid module attachment of existing names.  Propagate
> 	defining module for new 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-10_d.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-13_f.C: New test.
> 	* g++.dg/modules/tpl-friend-13_g.C: New test.
> 	* g++.dg/modules/tpl-friend-14_a.C: New test.
> 	* g++.dg/modules/tpl-friend-14_b.C: New test.
> 	* g++.dg/modules/tpl-friend-14_c.C: New test.
> 	* g++.dg/modules/tpl-friend-14_d.C: New test.
> 	* g++.dg/modules/tpl-friend-9.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> Reviewed-by: Jason Merrill <jason@redhat.com>
> Reviewed-by: Patrick Palka <ppalka@redhat.com>
> ---
>  gcc/cp/cp-tree.h                              |  3 +
>  gcc/cp/decl.cc                                | 41 ++++++----
>  gcc/cp/module.cc                              | 75 +++++++++++++++++++
>  gcc/cp/name-lookup.cc                         | 53 +++++++++++++
>  gcc/cp/pt.cc                                  | 32 +++++++-
>  .../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-10_d.C          |  8 ++
>  .../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          | 13 ++++
>  .../g++.dg/modules/tpl-friend-13_b.C          | 11 +++
>  .../g++.dg/modules/tpl-friend-13_c.C          | 13 ++++
>  .../g++.dg/modules/tpl-friend-13_d.C          |  7 ++
>  .../g++.dg/modules/tpl-friend-13_e.C          | 18 +++++
>  .../g++.dg/modules/tpl-friend-13_f.C          |  7 ++
>  .../g++.dg/modules/tpl-friend-13_g.C          | 11 +++
>  .../g++.dg/modules/tpl-friend-14_a.C          |  8 ++
>  .../g++.dg/modules/tpl-friend-14_b.C          |  8 ++
>  .../g++.dg/modules/tpl-friend-14_c.C          |  7 ++
>  .../g++.dg/modules/tpl-friend-14_d.C          |  9 +++
>  gcc/testsuite/g++.dg/modules/tpl-friend-9.C   | 13 ++++
>  29 files changed, 418 insertions(+), 17 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-10_d.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-13_f.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-13_g.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-14_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-14_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-14_c.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-14_d.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 9975dc78456..0c14241fce7 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7417,6 +7417,8 @@ 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 remove_defining_module (tree decl);
>  
>  extern void mangle_module (int m, bool include_partition);
>  extern void mangle_module_fini ();
> @@ -7650,6 +7652,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 91268ff631d..8fd39957fdc 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -2276,30 +2276,35 @@ 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))
> +	  /* The old declaration should match the exportingness of the new
> +	     declaration.  But hidden friend declarations just keep the
> +	     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 (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.  */
> @@ -3321,6 +3326,10 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
>    if (flag_concepts)
>      remove_constraints (newdecl);
>  
> +  /* And similarly for any module tracking data.  */
> +  if (modules_p ())
> +    remove_defining_module (newdecl);
> +
>    ggc_free (newdecl);
>  
>    return olddecl;
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index c2f077d6fd8..85c410aaa4c 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -2727,6 +2727,12 @@ 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 attached to the same module as the temploid.  This maps
> +   these decls to the temploid they are instantiated them, as there is
> +   no other easy way to get this information.  */
> +static GTY((cache)) decl_tree_cache_map *imported_temploid_friends;
> +
>  /********************************************************************/
>  /* Tree streaming.   The tree streaming is very specific to the tree
>     structures themselves.  A tag indicates the kind of tree being
> @@ -7820,6 +7826,12 @@ trees_out::decl_value (tree decl, depset *dep)
>  		  && DECL_MODULE_ATTACH_P (not_tmpl))
>  		is_attached = true;
>  
> +	      /* But don't consider imported temploid friends as attached,
> +		 since importers will need to merge this decl even if it was
> +		 attached to a different module.  */
> +	      if (imported_temploid_friends->get (decl))
> +		is_attached = false;
> +
>  	      bits.b (is_attached);
>  	    }
>  	  bits.b (dep && dep->has_defn ());
> @@ -7997,6 +8009,15 @@ trees_out::decl_value (tree decl, depset *dep)
>  	}
>      }
>  
> +  if (TREE_CODE (inner) == FUNCTION_DECL
> +      || TREE_CODE (inner) == TYPE_DECL)
> +    {
> +      /* 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 +8324,12 @@ trees_in::decl_value ()
>  	}
>      }
>  
> +  if (TREE_CODE (inner) == FUNCTION_DECL
> +      || TREE_CODE (inner) == TYPE_DECL)
> +    if (tree owner = tree_node ())
> +      if (is_new)
> +	imported_temploid_friends->put (decl, owner);
> +
>    /* Regular typedefs will have a NULL TREE_TYPE at this point.  */
>    unsigned tdef_flags = 0;
>    bool is_typedef = false;
> @@ -18941,6 +18968,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))
>  	{
> @@ -19249,6 +19282,46 @@ 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);
> +    }
> +}
> +
> +/* DECL is being freed, clear data we don't need anymore.  */
> +
> +void
> +remove_defining_module (tree decl)
> +{
> +  if (!modules_p ())
> +    return;
> +
> +  if (imported_temploid_friends)
> +    imported_temploid_friends->remove (decl);
> +}
> +
>  /* Create the flat name string.  It is simplest to have it handy.  */
>  
>  void
> @@ -20462,6 +20535,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
> +	= decl_tree_cache_map::create_ggc (EXPERIMENT (1, 400));
>      }
>  
>  #if CHECKING_P
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 7af7f00e34c..4dffc0e9acc 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -4131,6 +4131,22 @@ mergeable_namespace_slots (tree ns, tree name, bool is_attached, tree *vec)
>    return vslot;
>  }
>  
> +/* Retrieve the bindings for an existing mergeable entity in namespace
> +   NS slot NAME.  Returns NULL if no such bindings exists.  */
> +
> +static tree
> +get_mergeable_namespace_binding (tree ns, tree name, bool is_attached)
> +{
> +  tree *mslot = find_namespace_slot (ns, name, false);
> +  if (!mslot || !*mslot || TREE_CODE (*mslot) != BINDING_VECTOR)
> +    return NULL_TREE;
> +
> +  tree *vslot = get_fixed_binding_slot
> +    (mslot, name, is_attached ? BINDING_SLOT_PARTITION : BINDING_SLOT_GLOBAL,
> +     false);
> +  return vslot ? *vslot : NULL_TREE;
> +}
> +
>  /* DECL is a new mergeable namespace-scope decl.  Add it to the
>     mergeable entities on GSLOT.  */
>  
> @@ -4453,6 +4469,43 @@ push_local_binding (tree id, tree decl, bool is_using)
>    add_decl_to_level (b, decl);
>  }
>  
> +/* Lookup the FRIEND_TMPL within all merged module imports.  Used to dedup
> +   instantiations of temploid hidden friends from imported modules.  */
> +
> +tree
> +lookup_imported_hidden_friend (tree friend_tmpl)
> +{
> +  /* For a class-scope friend class it should have been found by regular
> +     name lookup.  Otherwise we're looking in the current namespace.  */
> +  gcc_checking_assert (CP_DECL_CONTEXT (friend_tmpl) == current_namespace);
> +
> +  tree inner = DECL_TEMPLATE_RESULT (friend_tmpl);
> +  if (!DECL_LANG_SPECIFIC (inner)
> +      || !DECL_MODULE_IMPORT_P (inner))
> +    return NULL_TREE;
> +
> +  /* Imported temploid friends are not considered as attached to this
> +     module for merging purposes.  */
> +  tree bind = get_mergeable_namespace_binding (current_namespace,
> +					       DECL_NAME (inner), false);
> +  if (!bind)
> +    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);
> +
> +  /* There should be at most one class template from the module we're
> +     looking for, return it.  */
> +  for (ovl_iterator iter (bind); iter; ++iter)
> +    if (DECL_CLASS_TEMPLATE_P (*iter)
> +	&& get_originating_module (*iter) == m)
> +      return *iter;
> +
> +  return NULL_TREE;
> +}
> +
>  \f
>  /* 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..ac7669a7ffb 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,12 @@ 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 the existing declaration's module attachment is
> +	   compatible with the attachment of the friend template.  */
> +	module_may_redeclare (tmpl, friend_tmpl);
> +
>        if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (friend_tmpl))
>  	  > TMPL_ARGS_DEPTH (args))
>  	{
> @@ -11751,9 +11767,19 @@ tsubst_friend_class (tree friend_tmpl, tree args)
>        if (tmpl != error_mark_node)
>  	{
>  	  /* The new TMPL is not an instantiation of anything, so we
> -	     forget its origins.  We don't reset CLASSTYPE_TI_TEMPLATE
> +	     forget its origins.  It is also not a specialization of
> +	     anything.  We don't reset CLASSTYPE_TI_TEMPLATE
>  	     for the new type because that is supposed to be the
>  	     corresponding template decl, i.e., TMPL.  */
> +	  if (modules_p ())
> +	    {
> +	      spec_entry elt;
> +	      elt.tmpl = friend_tmpl;
> +	      elt.args = CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl));
> +	      elt.spec = TREE_TYPE (tmpl);
> +	      type_specializations->remove_elt (&elt);
> +	    }
> +
>  	  DECL_USE_TEMPLATE (tmpl) = 0;
>  	  DECL_TEMPLATE_INFO (tmpl) = NULL_TREE;
>  	  CLASSTYPE_USE_TEMPLATE (TREE_TYPE (tmpl)) = 0;
> @@ -11772,6 +11798,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..90bcd18a45e
> --- /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:part }
> +
> +export module user:part;
> +import test_support; 
> +export A<int> b;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-10_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-10_d.C
> new file mode 100644
> index 00000000000..861d19c9eaa
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-10_d.C
> @@ -0,0 +1,8 @@
> +// PR c++/105320
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi user }
> +
> +export module user;
> +export import :part;
> +import test_support; 
> +A<double> c;
> 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..8c972776d60
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_a.C
> @@ -0,0 +1,13 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +export template <typename> struct A {
> +  friend struct S;
> +  template <typename> friend struct T;
> +};
> +
> +export template <typename> struct B {
> +  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..0e27e97b113
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_b.C
> @@ -0,0 +1,11 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +A<int> a;
> +struct S {};  // { dg-error "conflicts with import" }
> +template <typename> struct T {};  // { dg-error "conflicts with import" }
> +
> +B<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..3464aa26bf8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_c.C
> @@ -0,0 +1,13 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +struct S {};  // { dg-error "conflicts with import" }
> +template <typename> struct T {};  // { 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" }
> +B<int> b;  // { 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..5b935474ab2
> --- /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 X }
> +
> +export module X;
> +export import M;
> +A<int> ax;
> +B<int> bx;
> 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..afbd0a39c23
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> @@ -0,0 +1,18 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +// 'import X' does not correctly notice that S has already been declared.
> +struct S {};  // { dg-message "previously declared" "" { xfail *-*-* } }
> +template <typename> struct T {};  // { dg-message "previously declared" }
> +void f() {}  // { dg-message "previously declared" }
> +template <typename T> void g() {}  // { dg-message "previously declared" }
> +
> +import X;
> +A<double> a2;  // { dg-message "required from here" }
> +B<double> b2;  // { dg-message "required from here" }
> +
> +// specifically, S and T are defined in M, not X, despite the instantiation being in X
> +// { dg-error "conflicting declaration \[^\n\r\]* S@M" "" { xfail *-*-* } 0 }
> +// { dg-error "conflicting declaration \[^\n\r\]* T@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-13_f.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_f.C
> new file mode 100644
> index 00000000000..287f95c7bdc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_f.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi Y }
> +
> +export module Y;
> +export import M;
> +A<double> ay;
> +B<double> by;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_g.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_g.C
> new file mode 100644
> index 00000000000..b7da60f2322
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_g.C
> @@ -0,0 +1,11 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import X;
> +import Y;
> +
> +// This should happily refer to the same S and T
> +// as already instantiated in both X and Y
> +A<long> az;
> +
> +// And same for f and g
> +B<long> bz;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-14_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-14_a.C
> new file mode 100644
> index 00000000000..6912512ecf7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-14_a.C
> @@ -0,0 +1,8 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +
> +export extern "C++" template <typename> struct A {
> +  template <typename> friend struct B;
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-14_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-14_b.C
> new file mode 100644
> index 00000000000..5f8aa7fd62e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-14_b.C
> @@ -0,0 +1,8 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi X }
> +
> +export module X;
> +export import M;
> +
> +A<int> x;
> +export extern "C++" template <typename T> struct B { using type = T; };
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-14_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-14_c.C
> new file mode 100644
> index 00000000000..8d89298878a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-14_c.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi Y }
> +
> +export module Y;
> +export import M;
> +
> +A<double> x;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-14_d.C b/gcc/testsuite/g++.dg/modules/tpl-friend-14_d.C
> new file mode 100644
> index 00000000000..7a842586b62
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-14_d.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import X;
> +import Y;
> +
> +int main() {
> +  A<long> a;
> +  B<int>::type r = 10;
> +}
> 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
> 
>