diff mbox series

c++/modules: Fix merging of GM entities in partitions [PR114950]

Message ID 66b0d0a6.170a0220.150af6.1381@mx.google.com
State New
Headers show
Series c++/modules: Fix merging of GM entities in partitions [PR114950] | expand

Commit Message

Nathaniel Shead Aug. 5, 2024, 1:16 p.m. UTC 
Bootstrapped and regtested (so far just modules.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest passes?

-- >8 --

Currently name lookup generally seems to assume that all entities
declared within a named module (partition) are attached to said module,
which is not true for GM entities (e.g. via extern "C++"), and causes
issues with deduplication.

This patch fixes the issue by ensuring that module attachment of a
declaration is consistently used to handling merging.  Handling this
exposes some issues with deduplicating temploid friends; to resolve this
we always create the BINDING_SLOT_PARTITION slot so that we have
somewhere to place attached names (from any module).

	PR c++/114950

gcc/cp/ChangeLog:

	* module.cc (trees_out::decl_value): Stream bit indicating
	imported temploid friends early.
	(trees_in::decl_value): Use this bit with key_mergeable.
	(trees_in::key_mergeable): Allow merging attached declarations
	if they're imported temploid friends.
	(module_state::read_cluster): Check for GM entities that may
	require merging even when importing from partitions.
	* name-lookup.cc (enum binding_slots): Adjust comment.
	(get_fixed_binding_slot): Always create partition slot.
	(name_lookup::search_namespace_only): Support binding vectors
	with both partition and GM entities to dedup.
	(walk_module_binding): Likewise.
	(name_lookup::adl_namespace_fns): Likewise.
	(set_module_binding): Likewise.
	(check_module_override): Use attachment of the decl when
	checking overrides rather than named_module_p.
	(lookup_imported_hidden_friend): Use partition slot for finding
	mergeable template bindings.
	* name-lookup.h (set_module_binding): Split mod_glob_flag
	parameter into separate global_p and partition_p params.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/tpl-friend-13_e.C: Adjust error message.
	* g++.dg/modules/ambig-2_a.C: New test.
	* g++.dg/modules/ambig-2_b.C: New test.
	* g++.dg/modules/part-9_a.C: New test.
	* g++.dg/modules/part-9_b.C: New test.
	* g++.dg/modules/part-9_c.C: New test.
	* g++.dg/modules/tpl-friend-15.h: New test.
	* g++.dg/modules/tpl-friend-15_a.C: New test.
	* g++.dg/modules/tpl-friend-15_b.C: New test.
	* g++.dg/modules/tpl-friend-15_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                              | 55 ++++++++++------
 gcc/cp/name-lookup.cc                         | 65 ++++++++++---------
 gcc/cp/name-lookup.h                          |  2 +-
 gcc/testsuite/g++.dg/modules/ambig-2_a.C      |  7 ++
 gcc/testsuite/g++.dg/modules/ambig-2_b.C      | 10 +++
 gcc/testsuite/g++.dg/modules/part-9_a.C       | 10 +++
 gcc/testsuite/g++.dg/modules/part-9_b.C       | 10 +++
 gcc/testsuite/g++.dg/modules/part-9_c.C       |  8 +++
 .../g++.dg/modules/tpl-friend-13_e.C          |  4 +-
 gcc/testsuite/g++.dg/modules/tpl-friend-15.h  | 11 ++++
 .../g++.dg/modules/tpl-friend-15_a.C          |  8 +++
 .../g++.dg/modules/tpl-friend-15_b.C          |  8 +++
 .../g++.dg/modules/tpl-friend-15_c.C          |  7 ++
 13 files changed, 150 insertions(+), 55 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/part-9_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/part-9_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/part-9_c.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15.h
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C

Comments

Jason Merrill Aug. 7, 2024, 8:18 p.m. UTC | #1 
On 8/5/24 9:16 AM, Nathaniel Shead wrote:
> Bootstrapped and regtested (so far just modules.exp) on
> x86_64-pc-linux-gnu, OK for trunk if full regtest passes?

OK.

> -- >8 --
> 
> Currently name lookup generally seems to assume that all entities
> declared within a named module (partition) are attached to said module,
> which is not true for GM entities (e.g. via extern "C++"), and causes
> issues with deduplication.
> 
> This patch fixes the issue by ensuring that module attachment of a
> declaration is consistently used to handling merging.  Handling this
> exposes some issues with deduplicating temploid friends; to resolve this
> we always create the BINDING_SLOT_PARTITION slot so that we have
> somewhere to place attached names (from any module).
> 
> 	PR c++/114950
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::decl_value): Stream bit indicating
> 	imported temploid friends early.
> 	(trees_in::decl_value): Use this bit with key_mergeable.
> 	(trees_in::key_mergeable): Allow merging attached declarations
> 	if they're imported temploid friends.
> 	(module_state::read_cluster): Check for GM entities that may
> 	require merging even when importing from partitions.
> 	* name-lookup.cc (enum binding_slots): Adjust comment.
> 	(get_fixed_binding_slot): Always create partition slot.
> 	(name_lookup::search_namespace_only): Support binding vectors
> 	with both partition and GM entities to dedup.
> 	(walk_module_binding): Likewise.
> 	(name_lookup::adl_namespace_fns): Likewise.
> 	(set_module_binding): Likewise.
> 	(check_module_override): Use attachment of the decl when
> 	checking overrides rather than named_module_p.
> 	(lookup_imported_hidden_friend): Use partition slot for finding
> 	mergeable template bindings.
> 	* name-lookup.h (set_module_binding): Split mod_glob_flag
> 	parameter into separate global_p and partition_p params.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/tpl-friend-13_e.C: Adjust error message.
> 	* g++.dg/modules/ambig-2_a.C: New test.
> 	* g++.dg/modules/ambig-2_b.C: New test.
> 	* g++.dg/modules/part-9_a.C: New test.
> 	* g++.dg/modules/part-9_b.C: New test.
> 	* g++.dg/modules/part-9_c.C: New test.
> 	* g++.dg/modules/tpl-friend-15.h: New test.
> 	* g++.dg/modules/tpl-friend-15_a.C: New test.
> 	* g++.dg/modules/tpl-friend-15_b.C: New test.
> 	* g++.dg/modules/tpl-friend-15_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                              | 55 ++++++++++------
>   gcc/cp/name-lookup.cc                         | 65 ++++++++++---------
>   gcc/cp/name-lookup.h                          |  2 +-
>   gcc/testsuite/g++.dg/modules/ambig-2_a.C      |  7 ++
>   gcc/testsuite/g++.dg/modules/ambig-2_b.C      | 10 +++
>   gcc/testsuite/g++.dg/modules/part-9_a.C       | 10 +++
>   gcc/testsuite/g++.dg/modules/part-9_b.C       | 10 +++
>   gcc/testsuite/g++.dg/modules/part-9_c.C       |  8 +++
>   .../g++.dg/modules/tpl-friend-13_e.C          |  4 +-
>   gcc/testsuite/g++.dg/modules/tpl-friend-15.h  | 11 ++++
>   .../g++.dg/modules/tpl-friend-15_a.C          |  8 +++
>   .../g++.dg/modules/tpl-friend-15_b.C          |  8 +++
>   .../g++.dg/modules/tpl-friend-15_c.C          |  7 ++
>   13 files changed, 150 insertions(+), 55 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/part-9_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/part-9_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/part-9_c.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15.h
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index d1607a06757..e6b569ebca5 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -2955,7 +2955,8 @@ private:
>   public:
>     tree decl_container ();
>     tree key_mergeable (int tag, merge_kind, tree decl, tree inner, tree type,
> -		      tree container, bool is_attached);
> +		      tree container, bool is_attached,
> +		      bool is_imported_temploid_friend);
>     unsigned binfo_mergeable (tree *);
>   
>   private:
> @@ -7803,6 +7804,7 @@ trees_out::decl_value (tree decl, depset *dep)
>   		       || !TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)));
>   
>     merge_kind mk = get_merge_kind (decl, dep);
> +  bool is_imported_temploid_friend = imported_temploid_friends->get (decl);
>   
>     if (CHECKING_P)
>       {
> @@ -7838,13 +7840,11 @@ 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);
> +
> +	      /* Also tell the importer whether this is an imported temploid
> +		 friend, which has implications for merging.  */
> +	      bits.b (is_imported_temploid_friend);
>   	    }
>   	  bits.b (dep && dep->has_defn ());
>   	}
> @@ -8021,13 +8021,12 @@ trees_out::decl_value (tree decl, depset *dep)
>   	}
>       }
>   
> -  if (TREE_CODE (inner) == FUNCTION_DECL
> -      || TREE_CODE (inner) == TYPE_DECL)
> +  if (is_imported_temploid_friend)
>       {
>         /* 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);
> +      tree_node (*slot);
>       }
>   
>     bool is_typedef = false;
> @@ -8106,6 +8105,7 @@ trees_in::decl_value ()
>   {
>     int tag = 0;
>     bool is_attached = false;
> +  bool is_imported_temploid_friend = false;
>     bool has_defn = false;
>     unsigned mk_u = u ();
>     if (mk_u >= MK_hwm || !merge_kind_name[mk_u])
> @@ -8126,7 +8126,10 @@ trees_in::decl_value ()
>   	{
>   	  bits_in bits = stream_bits ();
>   	  if (!(mk & MK_template_mask) && !state->is_header ())
> -	    is_attached = bits.b ();
> +	    {
> +	      is_attached = bits.b ();
> +	      is_imported_temploid_friend = bits.b ();
> +	    }
>   
>   	  has_defn = bits.b ();
>   	}
> @@ -8231,7 +8234,7 @@ trees_in::decl_value ()
>       parm_tag = fn_parms_init (inner);
>   
>     tree existing = key_mergeable (tag, mk, decl, inner, type, container,
> -				 is_attached);
> +				 is_attached, is_imported_temploid_friend);
>     tree existing_inner = existing;
>     if (existing)
>       {
> @@ -8336,8 +8339,7 @@ trees_in::decl_value ()
>   	}
>       }
>   
> -  if (TREE_CODE (inner) == FUNCTION_DECL
> -      || TREE_CODE (inner) == TYPE_DECL)
> +  if (is_imported_temploid_friend)
>       if (tree owner = tree_node ())
>         if (is_new)
>   	imported_temploid_friends->put (decl, owner);
> @@ -11174,7 +11176,8 @@ check_mergeable_decl (merge_kind mk, tree decl, tree ovl, merge_key const &key)
>   
>   tree
>   trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> -			 tree type, tree container, bool is_attached)
> +			 tree type, tree container, bool is_attached,
> +			 bool is_imported_temploid_friend)
>   {
>     const char *kind = "new";
>     tree existing = NULL_TREE;
> @@ -11316,6 +11319,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
>   
>   	  case NAMESPACE_DECL:
>   	    if (is_attached
> +		&& !is_imported_temploid_friend

How can a namespace be an imported temploid friend?

>   		&& !(state->is_module () || state->is_partition ()))
>   	      kind = "unique";
>   	    else
> @@ -11347,7 +11351,9 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
>   	    break;
>   
>   	  case TYPE_DECL:
> -	    if (is_attached && !(state->is_module () || state->is_partition ())
> +	    if (is_attached
> +		&& !is_imported_temploid_friend
> +		&& !(state->is_module () || state->is_partition ())
>   		/* Implicit member functions can come from
>   		   anywhere.  */
>   		&& !(DECL_ARTIFICIAL (decl)
> @@ -15291,6 +15297,7 @@ module_state::read_cluster (unsigned snum)
>   	    tree visible = NULL_TREE;
>   	    tree type = NULL_TREE;
>   	    bool dedup = false;
> +	    bool global_p = false;
>   
>   	    /* We rely on the bindings being in the reverse order of
>   	       the resulting overload set.  */
> @@ -15308,6 +15315,16 @@ module_state::read_cluster (unsigned snum)
>   		if (sec.get_overrun ())
>   		  break;
>   
> +		if (!global_p)
> +		  {
> +		    /* Check if the decl could require GM merging.  */
> +		    tree orig = get_originating_module_decl (decl);
> +		    tree inner = STRIP_TEMPLATE (orig);
> +		    if (!DECL_LANG_SPECIFIC (inner)
> +			|| !DECL_MODULE_ATTACH_P (inner))
> +		      global_p = true;
> +		  }
> +
>   		if (decls && TREE_CODE (decl) == TYPE_DECL)
>   		  {
>   		    /* Stat hack.  */
> @@ -15394,10 +15411,8 @@ module_state::read_cluster (unsigned snum)
>   	      break; /* Bail.  */
>   
>   	    dump () && dump ("Binding of %P", ns, name);
> -	    if (!set_module_binding (ns, name, mod,
> -				     is_header () ? -1
> -				     : is_module () || is_partition () ? 1
> -				     : 0,
> +	    if (!set_module_binding (ns, name, mod, global_p,
> +				     is_module () || is_partition (),
>   				     decls, type, visible))
>   	      sec.set_overrun ();
>   	  }
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 8823ab71c60..872f1af0b2e 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -51,8 +51,8 @@ enum binding_slots
>   {
>    BINDING_SLOT_CURRENT,	/* Slot for current TU.  */
>    BINDING_SLOT_GLOBAL,	/* Slot for merged global module. */
> - BINDING_SLOT_PARTITION, /* Slot for merged partition entities
> -			    (optional).  */
> + BINDING_SLOT_PARTITION, /* Slot for merged partition entities or
> +			    imported friends.  */
>   
>    /* Number of always-allocated slots.  */
>    BINDING_SLOTS_FIXED = BINDING_SLOT_GLOBAL + 1
> @@ -248,9 +248,10 @@ get_fixed_binding_slot (tree *slot, tree name, unsigned ix, int create)
>         if (!create)
>   	return NULL;
>   
> -      /* The partition slot is only needed when we're a named
> -	 module.  */
> -      bool partition_slot = named_module_p ();
> +      /* The partition slot is always needed, in case we have imported
> +	 temploid friends with attachment different from the module we
> +	 imported them from.  */
> +      bool partition_slot = true;
>         unsigned want = ((BINDING_SLOTS_FIXED + partition_slot + (create < 0)
>   			+ BINDING_VECTOR_SLOTS_PER_CLUSTER - 1)
>   		       / BINDING_VECTOR_SLOTS_PER_CLUSTER);
> @@ -937,7 +938,6 @@ name_lookup::search_namespace_only (tree scope)
>   		   stat_hack, then everything was exported.  */
>   		tree type = NULL_TREE;
>   
> -
>   		/* If STAT_HACK_P is false, everything is visible, and
>   		   there's no duplication possibilities.  */
>   		if (STAT_HACK_P (bind))
> @@ -947,9 +947,9 @@ name_lookup::search_namespace_only (tree scope)
>   			/* Do we need to engage deduplication?  */
>   			int dup = 0;
>   			if (MODULE_BINDING_GLOBAL_P (bind))
> -			  dup = 1;
> -			else if (MODULE_BINDING_PARTITION_P (bind))
> -			  dup = 2;
> +			  dup |= 1;
> +			if (MODULE_BINDING_PARTITION_P (bind))
> +			  dup |= 2;
>   			if (unsigned hit = dup_detect & dup)
>   			  {
>   			    if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
> @@ -1275,9 +1275,9 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
>   			/* Do we need to engage deduplication?  */
>   			int dup = 0;
>   			if (MODULE_BINDING_GLOBAL_P (bind))
> -			  dup = 1;
> -			else if (MODULE_BINDING_PARTITION_P (bind))
> -			  dup = 2;
> +			  dup |= 1;
> +			if (MODULE_BINDING_PARTITION_P (bind))
> +			  dup |= 2;
>   			if (unsigned hit = dup_detect & dup)
>   			  if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
>   			      || (hit & 2
> @@ -3758,6 +3758,9 @@ check_module_override (tree decl, tree mvec, bool hiding,
>     binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (mvec);
>     unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (mvec);
>   
> +  tree nontmpl = STRIP_TEMPLATE (decl);
> +  bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
> +
>     if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
>       {
>         cluster++;
> @@ -3819,7 +3822,7 @@ check_module_override (tree decl, tree mvec, bool hiding,
>       {
>         /* Look in the appropriate mergeable decl slot.  */
>         tree mergeable = NULL_TREE;
> -      if (named_module_p ())
> +      if (attached)
>   	mergeable = BINDING_VECTOR_CLUSTER (mvec, BINDING_SLOT_PARTITION
>   					   / BINDING_VECTOR_SLOTS_PER_CLUSTER)
>   	  .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER];
> @@ -3839,15 +3842,13 @@ check_module_override (tree decl, tree mvec, bool hiding,
>    matched:
>     if (match != error_mark_node)
>       {
> -      if (named_module_p ())
> +      if (attached)
>   	BINDING_VECTOR_PARTITION_DUPS_P (mvec) = true;
>         else
>   	BINDING_VECTOR_GLOBAL_DUPS_P (mvec) = true;
>       }
>   
>     return match;
> -
> -
>   }
>   
>   /* Record DECL as belonging to the current lexical scope.  Check for
> @@ -4300,7 +4301,9 @@ walk_module_binding (tree binding, bitmap partitions,
>   	  cluster++;
>   	}
>   
> -      bool maybe_dups = BINDING_VECTOR_PARTITION_DUPS_P (binding);
> +      /* There could be duplicate module or GMF entries.  */
> +      bool maybe_dups = (BINDING_VECTOR_PARTITION_DUPS_P (binding)
> +			 || BINDING_VECTOR_GLOBAL_DUPS_P (binding));
>   
>         for (; ix--; cluster++)
>   	for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
> @@ -4394,14 +4397,14 @@ import_module_binding  (tree ns, tree name, unsigned mod, unsigned snum)
>   }
>   
>   /* An import of MODULE is binding NS::NAME.  There should be no
> -   existing binding for >= MODULE.  MOD_GLOB indicates whether MODULE
> -   is a header_unit (-1) or part of the current module (+1).  VALUE
> -   and TYPE are the value and type bindings. VISIBLE are the value
> -   bindings being exported.  */
> +   existing binding for >= MODULE.  GLOBAL_P indicates whether the
> +   bindings include global module entities.  PARTITION_P is true if
> +   it is part of the current module. VALUE and TYPE are the value
> +   and type bindings. VISIBLE are the value bindings being exported.  */
>   
>   bool
> -set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
> -		    tree value, tree type, tree visible)
> +set_module_binding (tree ns, tree name, unsigned mod, bool global_p,
> +		    bool partition_p, tree value, tree type, tree visible)
>   {
>     if (!value)
>       /* Bogus BMIs could give rise to nothing to bind.  */
> @@ -4419,19 +4422,19 @@ set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
>       return false;
>   
>     tree bind = value;
> -  if (type || visible != bind || mod_glob)
> +  if (type || visible != bind || partition_p || global_p)
>       {
>         bind = stat_hack (bind, type);
>         STAT_VISIBLE (bind) = visible;
> -      if ((mod_glob > 0 && TREE_PUBLIC (ns))
> +      if ((partition_p && TREE_PUBLIC (ns))
>   	  || (type && DECL_MODULE_EXPORT_P (type)))
>   	STAT_TYPE_VISIBLE_P (bind) = true;
>       }
>   
> -  /* Note if this is this-module or global binding.  */
> -  if (mod_glob > 0)
> +  /* Note if this is this-module and/or global binding.  */
> +  if (partition_p)
>       MODULE_BINDING_PARTITION_P (bind) = true;
> -  else if (mod_glob < 0)
> +  if (global_p)
>       MODULE_BINDING_GLOBAL_P (bind) = true;
>   
>     *mslot = bind;
> @@ -4540,10 +4543,8 @@ lookup_imported_hidden_friend (tree friend_tmpl)
>         || !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);
> +  tree bind = get_mergeable_namespace_binding
> +    (current_namespace, DECL_NAME (inner), DECL_MODULE_ATTACH_P (inner));
>     if (!bind)
>       return NULL_TREE;
>   
> diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
> index 5cf6ae6374a..7c4193444dd 100644
> --- a/gcc/cp/name-lookup.h
> +++ b/gcc/cp/name-lookup.h
> @@ -484,7 +484,7 @@ extern tree lookup_class_binding (tree ctx, tree name);
>   extern bool import_module_binding (tree ctx, tree name, unsigned mod,
>   				   unsigned snum);
>   extern bool set_module_binding (tree ctx, tree name, unsigned mod,
> -				int mod_glob_flag,
> +				bool global_p, bool partition_p,
>   				tree value, tree type, tree visible);
>   extern void add_module_namespace_decl (tree ns, tree decl);
>   
> diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_a.C b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
> new file mode 100644
> index 00000000000..d5dcc93584a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
> @@ -0,0 +1,7 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi A }
> +
> +export module A;
> +
> +extern "C++" int foo ();
> +extern "C++" char bar ();
> diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_b.C b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
> new file mode 100644
> index 00000000000..b94416aabbf
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
> @@ -0,0 +1,10 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi !B }
> +
> +export module B;
> +import A;
> +
> +extern "C++" int foo ();
> +extern "C++" int bar ();  // { dg-error "ambiguating new declaration" }
> +
> +// { dg-prune-output "not writing module" }
> diff --git a/gcc/testsuite/g++.dg/modules/part-9_a.C b/gcc/testsuite/g++.dg/modules/part-9_a.C
> new file mode 100644
> index 00000000000..dc033d301ee
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/part-9_a.C
> @@ -0,0 +1,10 @@
> +// PR c++/114950
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M:a }
> +
> +module M:a;
> +
> +struct A {};
> +extern "C++" struct B {};
> +void f(int) {}
> +extern "C++" void f(double) {}
> diff --git a/gcc/testsuite/g++.dg/modules/part-9_b.C b/gcc/testsuite/g++.dg/modules/part-9_b.C
> new file mode 100644
> index 00000000000..7339da22d97
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/part-9_b.C
> @@ -0,0 +1,10 @@
> +// PR c++/114950
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M:b }
> +
> +module M:b;
> +
> +struct A {};
> +extern "C++" struct B {};
> +void f(int) {}
> +extern "C++" void f(double) {}
> diff --git a/gcc/testsuite/g++.dg/modules/part-9_c.C b/gcc/testsuite/g++.dg/modules/part-9_c.C
> new file mode 100644
> index 00000000000..26ac6777b7a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/part-9_c.C
> @@ -0,0 +1,8 @@
> +// PR c++/114950
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +// Handle merging definitions of extern "C++" decls across partitions
> +
> +export module M;
> +import :a;
> +import :b;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> index afbd0a39c23..b32fd98b756 100644
> --- a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> @@ -1,8 +1,8 @@
>   // { 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" }
> +struct S {};  // { dg-message "previous declaration" "" { xfail *-*-* } }
> +template <typename> struct T {};  // { dg-message "previous declaration" }
>   void f() {}  // { dg-message "previously declared" }
>   template <typename T> void g() {}  // { dg-message "previously declared" }
>   
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15.h b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
> new file mode 100644
> index 00000000000..e4d3fff4445
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
> @@ -0,0 +1,11 @@
> +// PR c++/114950
> +
> +template <typename T>
> +struct A {
> +  friend void x();
> +};
> +template <typename T>
> +struct B {
> +  virtual void f() { A<T> r; }
> +};
> +template struct B<int>;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
> new file mode 100644
> index 00000000000..04c800875f4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
> @@ -0,0 +1,8 @@
> +// PR c++/114950
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M:a }
> +
> +module M:a;
> +extern "C++" {
> +  #include "tpl-friend-15.h"
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
> new file mode 100644
> index 00000000000..781882f97bc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
> @@ -0,0 +1,8 @@
> +// PR c++/114950
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M:b }
> +
> +module M:b;
> +extern "C++" {
> +  #include "tpl-friend-15.h"
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
> new file mode 100644
> index 00000000000..ced7e87d993
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
> @@ -0,0 +1,7 @@
> +// PR c++/114950
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi M }
> +
> +export module M;
> +import :a;
> +import :b;
Nathaniel Shead Aug. 7, 2024, 11:22 p.m. UTC | #2 
On Wed, Aug 07, 2024 at 04:18:47PM -0400, Jason Merrill wrote:
> On 8/5/24 9:16 AM, Nathaniel Shead wrote:
> > Bootstrapped and regtested (so far just modules.exp) on
> > x86_64-pc-linux-gnu, OK for trunk if full regtest passes?
> 
> OK.
> 
> > -- >8 --
> > 
> > Currently name lookup generally seems to assume that all entities
> > declared within a named module (partition) are attached to said module,
> > which is not true for GM entities (e.g. via extern "C++"), and causes
> > issues with deduplication.
> > 
> > This patch fixes the issue by ensuring that module attachment of a
> > declaration is consistently used to handling merging.  Handling this
> > exposes some issues with deduplicating temploid friends; to resolve this
> > we always create the BINDING_SLOT_PARTITION slot so that we have
> > somewhere to place attached names (from any module).
> > 
> > 	PR c++/114950
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* module.cc (trees_out::decl_value): Stream bit indicating
> > 	imported temploid friends early.
> > 	(trees_in::decl_value): Use this bit with key_mergeable.
> > 	(trees_in::key_mergeable): Allow merging attached declarations
> > 	if they're imported temploid friends.
> > 	(module_state::read_cluster): Check for GM entities that may
> > 	require merging even when importing from partitions.
> > 	* name-lookup.cc (enum binding_slots): Adjust comment.
> > 	(get_fixed_binding_slot): Always create partition slot.
> > 	(name_lookup::search_namespace_only): Support binding vectors
> > 	with both partition and GM entities to dedup.
> > 	(walk_module_binding): Likewise.
> > 	(name_lookup::adl_namespace_fns): Likewise.
> > 	(set_module_binding): Likewise.
> > 	(check_module_override): Use attachment of the decl when
> > 	checking overrides rather than named_module_p.
> > 	(lookup_imported_hidden_friend): Use partition slot for finding
> > 	mergeable template bindings.
> > 	* name-lookup.h (set_module_binding): Split mod_glob_flag
> > 	parameter into separate global_p and partition_p params.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/tpl-friend-13_e.C: Adjust error message.
> > 	* g++.dg/modules/ambig-2_a.C: New test.
> > 	* g++.dg/modules/ambig-2_b.C: New test.
> > 	* g++.dg/modules/part-9_a.C: New test.
> > 	* g++.dg/modules/part-9_b.C: New test.
> > 	* g++.dg/modules/part-9_c.C: New test.
> > 	* g++.dg/modules/tpl-friend-15.h: New test.
> > 	* g++.dg/modules/tpl-friend-15_a.C: New test.
> > 	* g++.dg/modules/tpl-friend-15_b.C: New test.
> > 	* g++.dg/modules/tpl-friend-15_c.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >   gcc/cp/module.cc                              | 55 ++++++++++------
> >   gcc/cp/name-lookup.cc                         | 65 ++++++++++---------
> >   gcc/cp/name-lookup.h                          |  2 +-
> >   gcc/testsuite/g++.dg/modules/ambig-2_a.C      |  7 ++
> >   gcc/testsuite/g++.dg/modules/ambig-2_b.C      | 10 +++
> >   gcc/testsuite/g++.dg/modules/part-9_a.C       | 10 +++
> >   gcc/testsuite/g++.dg/modules/part-9_b.C       | 10 +++
> >   gcc/testsuite/g++.dg/modules/part-9_c.C       |  8 +++
> >   .../g++.dg/modules/tpl-friend-13_e.C          |  4 +-
> >   gcc/testsuite/g++.dg/modules/tpl-friend-15.h  | 11 ++++
> >   .../g++.dg/modules/tpl-friend-15_a.C          |  8 +++
> >   .../g++.dg/modules/tpl-friend-15_b.C          |  8 +++
> >   .../g++.dg/modules/tpl-friend-15_c.C          |  7 ++
> >   13 files changed, 150 insertions(+), 55 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_a.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_b.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/part-9_a.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/part-9_b.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/part-9_c.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15.h
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index d1607a06757..e6b569ebca5 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -2955,7 +2955,8 @@ private:
> >   public:
> >     tree decl_container ();
> >     tree key_mergeable (int tag, merge_kind, tree decl, tree inner, tree type,
> > -		      tree container, bool is_attached);
> > +		      tree container, bool is_attached,
> > +		      bool is_imported_temploid_friend);
> >     unsigned binfo_mergeable (tree *);
> >   private:
> > @@ -7803,6 +7804,7 @@ trees_out::decl_value (tree decl, depset *dep)
> >   		       || !TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)));
> >     merge_kind mk = get_merge_kind (decl, dep);
> > +  bool is_imported_temploid_friend = imported_temploid_friends->get (decl);
> >     if (CHECKING_P)
> >       {
> > @@ -7838,13 +7840,11 @@ 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);
> > +
> > +	      /* Also tell the importer whether this is an imported temploid
> > +		 friend, which has implications for merging.  */
> > +	      bits.b (is_imported_temploid_friend);
> >   	    }
> >   	  bits.b (dep && dep->has_defn ());
> >   	}
> > @@ -8021,13 +8021,12 @@ trees_out::decl_value (tree decl, depset *dep)
> >   	}
> >       }
> > -  if (TREE_CODE (inner) == FUNCTION_DECL
> > -      || TREE_CODE (inner) == TYPE_DECL)
> > +  if (is_imported_temploid_friend)
> >       {
> >         /* 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);
> > +      tree_node (*slot);
> >       }
> >     bool is_typedef = false;
> > @@ -8106,6 +8105,7 @@ trees_in::decl_value ()
> >   {
> >     int tag = 0;
> >     bool is_attached = false;
> > +  bool is_imported_temploid_friend = false;
> >     bool has_defn = false;
> >     unsigned mk_u = u ();
> >     if (mk_u >= MK_hwm || !merge_kind_name[mk_u])
> > @@ -8126,7 +8126,10 @@ trees_in::decl_value ()
> >   	{
> >   	  bits_in bits = stream_bits ();
> >   	  if (!(mk & MK_template_mask) && !state->is_header ())
> > -	    is_attached = bits.b ();
> > +	    {
> > +	      is_attached = bits.b ();
> > +	      is_imported_temploid_friend = bits.b ();
> > +	    }
> >   	  has_defn = bits.b ();
> >   	}
> > @@ -8231,7 +8234,7 @@ trees_in::decl_value ()
> >       parm_tag = fn_parms_init (inner);
> >     tree existing = key_mergeable (tag, mk, decl, inner, type, container,
> > -				 is_attached);
> > +				 is_attached, is_imported_temploid_friend);
> >     tree existing_inner = existing;
> >     if (existing)
> >       {
> > @@ -8336,8 +8339,7 @@ trees_in::decl_value ()
> >   	}
> >       }
> > -  if (TREE_CODE (inner) == FUNCTION_DECL
> > -      || TREE_CODE (inner) == TYPE_DECL)
> > +  if (is_imported_temploid_friend)
> >       if (tree owner = tree_node ())
> >         if (is_new)
> >   	imported_temploid_friends->put (decl, owner);
> > @@ -11174,7 +11176,8 @@ check_mergeable_decl (merge_kind mk, tree decl, tree ovl, merge_key const &key)
> >   tree
> >   trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> > -			 tree type, tree container, bool is_attached)
> > +			 tree type, tree container, bool is_attached,
> > +			 bool is_imported_temploid_friend)
> >   {
> >     const char *kind = "new";
> >     tree existing = NULL_TREE;
> > @@ -11316,6 +11319,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> >   	  case NAMESPACE_DECL:
> >   	    if (is_attached
> > +		&& !is_imported_temploid_friend
> 
> How can a namespace be an imported temploid friend?
> 

Cut off by context, but this is

	switch (TREE_CODE (container))
	  {
	  default:
	    gcc_unreachable ();

	  case NAMESPACE_DECL:
	    if (is_attached
		&& !is_imported_temploid_friend
		&& !(state->is_module () || state->is_partition ()))
	      kind = "unique";

i.e. the NAMESPACE_DECL is referring to the container that the decl is
attached to for merging purposes.

> >   		&& !(state->is_module () || state->is_partition ()))
> >   	      kind = "unique";
> >   	    else
> > @@ -11347,7 +11351,9 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> >   	    break;
> >   	  case TYPE_DECL:
> > -	    if (is_attached && !(state->is_module () || state->is_partition ())
> > +	    if (is_attached
> > +		&& !is_imported_temploid_friend

This is the one that may perhaps be unnecessary (on thinking over this
again I would expect any class-scope friends to not be redeclared
outside of their named module, even for imported templates?), so I'll
actually re-test this patch without this hunk.

> > +		&& !(state->is_module () || state->is_partition ())
> >   		/* Implicit member functions can come from
> >   		   anywhere.  */
> >   		&& !(DECL_ARTIFICIAL (decl)
> > @@ -15291,6 +15297,7 @@ module_state::read_cluster (unsigned snum)
> >   	    tree visible = NULL_TREE;
> >   	    tree type = NULL_TREE;
> >   	    bool dedup = false;
> > +	    bool global_p = false;
> >   	    /* We rely on the bindings being in the reverse order of
> >   	       the resulting overload set.  */
> > @@ -15308,6 +15315,16 @@ module_state::read_cluster (unsigned snum)
> >   		if (sec.get_overrun ())
> >   		  break;
> > +		if (!global_p)
> > +		  {
> > +		    /* Check if the decl could require GM merging.  */
> > +		    tree orig = get_originating_module_decl (decl);
> > +		    tree inner = STRIP_TEMPLATE (orig);
> > +		    if (!DECL_LANG_SPECIFIC (inner)
> > +			|| !DECL_MODULE_ATTACH_P (inner))
> > +		      global_p = true;
> > +		  }
> > +
> >   		if (decls && TREE_CODE (decl) == TYPE_DECL)
> >   		  {
> >   		    /* Stat hack.  */
> > @@ -15394,10 +15411,8 @@ module_state::read_cluster (unsigned snum)
> >   	      break; /* Bail.  */
> >   	    dump () && dump ("Binding of %P", ns, name);
> > -	    if (!set_module_binding (ns, name, mod,
> > -				     is_header () ? -1
> > -				     : is_module () || is_partition () ? 1
> > -				     : 0,
> > +	    if (!set_module_binding (ns, name, mod, global_p,
> > +				     is_module () || is_partition (),
> >   				     decls, type, visible))
> >   	      sec.set_overrun ();
> >   	  }
> > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> > index 8823ab71c60..872f1af0b2e 100644
> > --- a/gcc/cp/name-lookup.cc
> > +++ b/gcc/cp/name-lookup.cc
> > @@ -51,8 +51,8 @@ enum binding_slots
> >   {
> >    BINDING_SLOT_CURRENT,	/* Slot for current TU.  */
> >    BINDING_SLOT_GLOBAL,	/* Slot for merged global module. */
> > - BINDING_SLOT_PARTITION, /* Slot for merged partition entities
> > -			    (optional).  */
> > + BINDING_SLOT_PARTITION, /* Slot for merged partition entities or
> > +			    imported friends.  */
> >    /* Number of always-allocated slots.  */
> >    BINDING_SLOTS_FIXED = BINDING_SLOT_GLOBAL + 1
> > @@ -248,9 +248,10 @@ get_fixed_binding_slot (tree *slot, tree name, unsigned ix, int create)
> >         if (!create)
> >   	return NULL;
> > -      /* The partition slot is only needed when we're a named
> > -	 module.  */
> > -      bool partition_slot = named_module_p ();
> > +      /* The partition slot is always needed, in case we have imported
> > +	 temploid friends with attachment different from the module we
> > +	 imported them from.  */
> > +      bool partition_slot = true;
> >         unsigned want = ((BINDING_SLOTS_FIXED + partition_slot + (create < 0)
> >   			+ BINDING_VECTOR_SLOTS_PER_CLUSTER - 1)
> >   		       / BINDING_VECTOR_SLOTS_PER_CLUSTER);
> > @@ -937,7 +938,6 @@ name_lookup::search_namespace_only (tree scope)
> >   		   stat_hack, then everything was exported.  */
> >   		tree type = NULL_TREE;
> > -
> >   		/* If STAT_HACK_P is false, everything is visible, and
> >   		   there's no duplication possibilities.  */
> >   		if (STAT_HACK_P (bind))
> > @@ -947,9 +947,9 @@ name_lookup::search_namespace_only (tree scope)
> >   			/* Do we need to engage deduplication?  */
> >   			int dup = 0;
> >   			if (MODULE_BINDING_GLOBAL_P (bind))
> > -			  dup = 1;
> > -			else if (MODULE_BINDING_PARTITION_P (bind))
> > -			  dup = 2;
> > +			  dup |= 1;
> > +			if (MODULE_BINDING_PARTITION_P (bind))
> > +			  dup |= 2;
> >   			if (unsigned hit = dup_detect & dup)
> >   			  {
> >   			    if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
> > @@ -1275,9 +1275,9 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
> >   			/* Do we need to engage deduplication?  */
> >   			int dup = 0;
> >   			if (MODULE_BINDING_GLOBAL_P (bind))
> > -			  dup = 1;
> > -			else if (MODULE_BINDING_PARTITION_P (bind))
> > -			  dup = 2;
> > +			  dup |= 1;
> > +			if (MODULE_BINDING_PARTITION_P (bind))
> > +			  dup |= 2;
> >   			if (unsigned hit = dup_detect & dup)
> >   			  if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
> >   			      || (hit & 2
> > @@ -3758,6 +3758,9 @@ check_module_override (tree decl, tree mvec, bool hiding,
> >     binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (mvec);
> >     unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (mvec);
> > +  tree nontmpl = STRIP_TEMPLATE (decl);
> > +  bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
> > +
> >     if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
> >       {
> >         cluster++;
> > @@ -3819,7 +3822,7 @@ check_module_override (tree decl, tree mvec, bool hiding,
> >       {
> >         /* Look in the appropriate mergeable decl slot.  */
> >         tree mergeable = NULL_TREE;
> > -      if (named_module_p ())
> > +      if (attached)
> >   	mergeable = BINDING_VECTOR_CLUSTER (mvec, BINDING_SLOT_PARTITION
> >   					   / BINDING_VECTOR_SLOTS_PER_CLUSTER)
> >   	  .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER];
> > @@ -3839,15 +3842,13 @@ check_module_override (tree decl, tree mvec, bool hiding,
> >    matched:
> >     if (match != error_mark_node)
> >       {
> > -      if (named_module_p ())
> > +      if (attached)
> >   	BINDING_VECTOR_PARTITION_DUPS_P (mvec) = true;
> >         else
> >   	BINDING_VECTOR_GLOBAL_DUPS_P (mvec) = true;
> >       }
> >     return match;
> > -
> > -
> >   }
> >   /* Record DECL as belonging to the current lexical scope.  Check for
> > @@ -4300,7 +4301,9 @@ walk_module_binding (tree binding, bitmap partitions,
> >   	  cluster++;
> >   	}
> > -      bool maybe_dups = BINDING_VECTOR_PARTITION_DUPS_P (binding);
> > +      /* There could be duplicate module or GMF entries.  */
> > +      bool maybe_dups = (BINDING_VECTOR_PARTITION_DUPS_P (binding)
> > +			 || BINDING_VECTOR_GLOBAL_DUPS_P (binding));
> >         for (; ix--; cluster++)
> >   	for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
> > @@ -4394,14 +4397,14 @@ import_module_binding  (tree ns, tree name, unsigned mod, unsigned snum)
> >   }
> >   /* An import of MODULE is binding NS::NAME.  There should be no
> > -   existing binding for >= MODULE.  MOD_GLOB indicates whether MODULE
> > -   is a header_unit (-1) or part of the current module (+1).  VALUE
> > -   and TYPE are the value and type bindings. VISIBLE are the value
> > -   bindings being exported.  */
> > +   existing binding for >= MODULE.  GLOBAL_P indicates whether the
> > +   bindings include global module entities.  PARTITION_P is true if
> > +   it is part of the current module. VALUE and TYPE are the value
> > +   and type bindings. VISIBLE are the value bindings being exported.  */
> >   bool
> > -set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
> > -		    tree value, tree type, tree visible)
> > +set_module_binding (tree ns, tree name, unsigned mod, bool global_p,
> > +		    bool partition_p, tree value, tree type, tree visible)
> >   {
> >     if (!value)
> >       /* Bogus BMIs could give rise to nothing to bind.  */
> > @@ -4419,19 +4422,19 @@ set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
> >       return false;
> >     tree bind = value;
> > -  if (type || visible != bind || mod_glob)
> > +  if (type || visible != bind || partition_p || global_p)
> >       {
> >         bind = stat_hack (bind, type);
> >         STAT_VISIBLE (bind) = visible;
> > -      if ((mod_glob > 0 && TREE_PUBLIC (ns))
> > +      if ((partition_p && TREE_PUBLIC (ns))
> >   	  || (type && DECL_MODULE_EXPORT_P (type)))
> >   	STAT_TYPE_VISIBLE_P (bind) = true;
> >       }
> > -  /* Note if this is this-module or global binding.  */
> > -  if (mod_glob > 0)
> > +  /* Note if this is this-module and/or global binding.  */
> > +  if (partition_p)
> >       MODULE_BINDING_PARTITION_P (bind) = true;
> > -  else if (mod_glob < 0)
> > +  if (global_p)
> >       MODULE_BINDING_GLOBAL_P (bind) = true;
> >     *mslot = bind;
> > @@ -4540,10 +4543,8 @@ lookup_imported_hidden_friend (tree friend_tmpl)
> >         || !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);
> > +  tree bind = get_mergeable_namespace_binding
> > +    (current_namespace, DECL_NAME (inner), DECL_MODULE_ATTACH_P (inner));
> >     if (!bind)
> >       return NULL_TREE;
> > diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
> > index 5cf6ae6374a..7c4193444dd 100644
> > --- a/gcc/cp/name-lookup.h
> > +++ b/gcc/cp/name-lookup.h
> > @@ -484,7 +484,7 @@ extern tree lookup_class_binding (tree ctx, tree name);
> >   extern bool import_module_binding (tree ctx, tree name, unsigned mod,
> >   				   unsigned snum);
> >   extern bool set_module_binding (tree ctx, tree name, unsigned mod,
> > -				int mod_glob_flag,
> > +				bool global_p, bool partition_p,
> >   				tree value, tree type, tree visible);
> >   extern void add_module_namespace_decl (tree ns, tree decl);
> > diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_a.C b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
> > new file mode 100644
> > index 00000000000..d5dcc93584a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
> > @@ -0,0 +1,7 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi A }
> > +
> > +export module A;
> > +
> > +extern "C++" int foo ();
> > +extern "C++" char bar ();
> > diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_b.C b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
> > new file mode 100644
> > index 00000000000..b94416aabbf
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
> > @@ -0,0 +1,10 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi !B }
> > +
> > +export module B;
> > +import A;
> > +
> > +extern "C++" int foo ();
> > +extern "C++" int bar ();  // { dg-error "ambiguating new declaration" }
> > +
> > +// { dg-prune-output "not writing module" }
> > diff --git a/gcc/testsuite/g++.dg/modules/part-9_a.C b/gcc/testsuite/g++.dg/modules/part-9_a.C
> > new file mode 100644
> > index 00000000000..dc033d301ee
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/part-9_a.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/114950
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M:a }
> > +
> > +module M:a;
> > +
> > +struct A {};
> > +extern "C++" struct B {};
> > +void f(int) {}
> > +extern "C++" void f(double) {}
> > diff --git a/gcc/testsuite/g++.dg/modules/part-9_b.C b/gcc/testsuite/g++.dg/modules/part-9_b.C
> > new file mode 100644
> > index 00000000000..7339da22d97
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/part-9_b.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/114950
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M:b }
> > +
> > +module M:b;
> > +
> > +struct A {};
> > +extern "C++" struct B {};
> > +void f(int) {}
> > +extern "C++" void f(double) {}
> > diff --git a/gcc/testsuite/g++.dg/modules/part-9_c.C b/gcc/testsuite/g++.dg/modules/part-9_c.C
> > new file mode 100644
> > index 00000000000..26ac6777b7a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/part-9_c.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/114950
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M }
> > +// Handle merging definitions of extern "C++" decls across partitions
> > +
> > +export module M;
> > +import :a;
> > +import :b;
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> > index afbd0a39c23..b32fd98b756 100644
> > --- a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
> > @@ -1,8 +1,8 @@
> >   // { 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" }
> > +struct S {};  // { dg-message "previous declaration" "" { xfail *-*-* } }
> > +template <typename> struct T {};  // { dg-message "previous declaration" }
> >   void f() {}  // { dg-message "previously declared" }
> >   template <typename T> void g() {}  // { dg-message "previously declared" }
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15.h b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
> > new file mode 100644
> > index 00000000000..e4d3fff4445
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
> > @@ -0,0 +1,11 @@
> > +// PR c++/114950
> > +
> > +template <typename T>
> > +struct A {
> > +  friend void x();
> > +};
> > +template <typename T>
> > +struct B {
> > +  virtual void f() { A<T> r; }
> > +};
> > +template struct B<int>;
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
> > new file mode 100644
> > index 00000000000..04c800875f4
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/114950
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M:a }
> > +
> > +module M:a;
> > +extern "C++" {
> > +  #include "tpl-friend-15.h"
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
> > new file mode 100644
> > index 00000000000..781882f97bc
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/114950
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M:b }
> > +
> > +module M:b;
> > +extern "C++" {
> > +  #include "tpl-friend-15.h"
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
> > new file mode 100644
> > index 00000000000..ced7e87d993
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
> > @@ -0,0 +1,7 @@
> > +// PR c++/114950
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi M }
> > +
> > +export module M;
> > +import :a;
> > +import :b;
>
Jason Merrill Aug. 8, 2024, 12:45 a.m. UTC | #3 
On 8/7/24 7:22 PM, Nathaniel Shead wrote:
> On Wed, Aug 07, 2024 at 04:18:47PM -0400, Jason Merrill wrote:
>> On 8/5/24 9:16 AM, Nathaniel Shead wrote:
>>> Bootstrapped and regtested (so far just modules.exp) on
>>> x86_64-pc-linux-gnu, OK for trunk if full regtest passes?
>>
>> OK.
>>
>>> @@ -11316,6 +11319,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
>>>    	  case NAMESPACE_DECL:
>>>    	    if (is_attached
>>> +		&& !is_imported_temploid_friend
>>
>> How can a namespace be an imported temploid friend?
> 
> Cut off by context, but this is
> 
> 	switch (TREE_CODE (container))
> 	  {
> 	  default:
> 	    gcc_unreachable ();
> 
> 	  case NAMESPACE_DECL:
> 	    if (is_attached
> 		&& !is_imported_temploid_friend
> 		&& !(state->is_module () || state->is_partition ()))
> 	      kind = "unique";
> 
> i.e. the NAMESPACE_DECL is referring to the container that the decl is
> attached to for merging purposes.

Oops, yes, I figured that out but forgot to delete that comment.  :)

>>>    		&& !(state->is_module () || state->is_partition ()))
>>>    	      kind = "unique";
>>>    	    else
>>> @@ -11347,7 +11351,9 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
>>>    	    break;
>>>    	  case TYPE_DECL:
>>> -	    if (is_attached && !(state->is_module () || state->is_partition ())
>>> +	    if (is_attached
>>> +		&& !is_imported_temploid_friend
> 
> This is the one that may perhaps be unnecessary (on thinking over this
> again I would expect any class-scope friends to not be redeclared
> outside of their named module, even for imported templates?), so I'll
> actually re-test this patch without this hunk.

Sounds good.

Jason
Nathaniel Shead Aug. 8, 2024, 2:20 a.m. UTC | #4 
On Wed, Aug 07, 2024 at 08:45:03PM -0400, Jason Merrill wrote:
> On 8/7/24 7:22 PM, Nathaniel Shead wrote:
> > On Wed, Aug 07, 2024 at 04:18:47PM -0400, Jason Merrill wrote:
> > > On 8/5/24 9:16 AM, Nathaniel Shead wrote:
> > > > Bootstrapped and regtested (so far just modules.exp) on
> > > > x86_64-pc-linux-gnu, OK for trunk if full regtest passes?
> > > 
> > > OK.
> > > 
> > > > @@ -11316,6 +11319,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> > > >    	  case NAMESPACE_DECL:
> > > >    	    if (is_attached
> > > > +		&& !is_imported_temploid_friend
> > > 
> > > How can a namespace be an imported temploid friend?
> > 
> > Cut off by context, but this is
> > 
> > 	switch (TREE_CODE (container))
> > 	  {
> > 	  default:
> > 	    gcc_unreachable ();
> > 
> > 	  case NAMESPACE_DECL:
> > 	    if (is_attached
> > 		&& !is_imported_temploid_friend
> > 		&& !(state->is_module () || state->is_partition ()))
> > 	      kind = "unique";
> > 
> > i.e. the NAMESPACE_DECL is referring to the container that the decl is
> > attached to for merging purposes.
> 
> Oops, yes, I figured that out but forgot to delete that comment.  :)
> 
> > > >    		&& !(state->is_module () || state->is_partition ()))
> > > >    	      kind = "unique";
> > > >    	    else
> > > > @@ -11347,7 +11351,9 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
> > > >    	    break;
> > > >    	  case TYPE_DECL:
> > > > -	    if (is_attached && !(state->is_module () || state->is_partition ())
> > > > +	    if (is_attached
> > > > +		&& !is_imported_temploid_friend
> > 
> > This is the one that may perhaps be unnecessary (on thinking over this
> > again I would expect any class-scope friends to not be redeclared
> > outside of their named module, even for imported templates?), so I'll
> > actually re-test this patch without this hunk.
> 
> Sounds good.
> 
> Jason
> 

This is what I'll push if full bootstrap+regtest succeeds, replacing
that hunk with a 'gcc_checking_assert (!is_imported_temploid_friend)'
just to be extra clear.

-- >8 --

Currently name lookup generally seems to assume that all entities
declared within a named module (partition) are attached to said module,
which is not true for GM entities (e.g. via extern "C++"), and causes
issues with deduplication.

This patch fixes the issue by ensuring that module attachment of a
declaration is consistently used to handling merging.  Handling this
exposes some issues with deduplicating temploid friends; to resolve this
we always create the BINDING_SLOT_PARTITION slot so that we have
somewhere to place attached names (from any module).

This doesn't yet completely handle issues with allowing otherwise
conflicting temploid friends from different modules to co-exist in the
same module if neither are reachable from the other via name lookup.

	PR c++/114950

gcc/cp/ChangeLog:

	* module.cc (trees_out::decl_value): Stream bit indicating
	imported temploid friends early.
	(trees_in::decl_value): Use this bit with key_mergeable.
	(trees_in::key_mergeable): Allow merging attached declarations
	if they're imported temploid friends (which must be namespace
	scope).
	(module_state::read_cluster): Check for GM entities that may
	require merging even when importing from partitions.
	* name-lookup.cc (enum binding_slots): Adjust comment.
	(get_fixed_binding_slot): Always create partition slot.
	(name_lookup::search_namespace_only): Support binding vectors
	with both partition and GM entities to dedup.
	(walk_module_binding): Likewise.
	(name_lookup::adl_namespace_fns): Likewise.
	(set_module_binding): Likewise.
	(check_module_override): Use attachment of the decl when
	checking overrides rather than named_module_p.
	(lookup_imported_hidden_friend): Use partition slot for finding
	mergeable template bindings.
	* name-lookup.h (set_module_binding): Split mod_glob_flag
	parameter into separate global_p and partition_p params.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/tpl-friend-13_e.C: Adjust error message.
	* g++.dg/modules/ambig-2_a.C: New test.
	* g++.dg/modules/ambig-2_b.C: New test.
	* g++.dg/modules/part-9_a.C: New test.
	* g++.dg/modules/part-9_b.C: New test.
	* g++.dg/modules/part-9_c.C: New test.
	* g++.dg/modules/tpl-friend-15.h: New test.
	* g++.dg/modules/tpl-friend-15_a.C: New test.
	* g++.dg/modules/tpl-friend-15_b.C: New test.
	* g++.dg/modules/tpl-friend-15_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                              | 52 +++++++++------
 gcc/cp/name-lookup.cc                         | 65 ++++++++++---------
 gcc/cp/name-lookup.h                          |  2 +-
 gcc/testsuite/g++.dg/modules/ambig-2_a.C      |  7 ++
 gcc/testsuite/g++.dg/modules/ambig-2_b.C      | 10 +++
 gcc/testsuite/g++.dg/modules/part-9_a.C       | 10 +++
 gcc/testsuite/g++.dg/modules/part-9_b.C       | 10 +++
 gcc/testsuite/g++.dg/modules/part-9_c.C       |  8 +++
 .../g++.dg/modules/tpl-friend-13_e.C          |  4 +-
 gcc/testsuite/g++.dg/modules/tpl-friend-15.h  | 11 ++++
 .../g++.dg/modules/tpl-friend-15_a.C          |  8 +++
 .../g++.dg/modules/tpl-friend-15_b.C          |  8 +++
 .../g++.dg/modules/tpl-friend-15_c.C          |  7 ++
 13 files changed, 148 insertions(+), 54 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/ambig-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/part-9_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/part-9_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/part-9_c.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15.h
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 723f0890d96..1a6f7a2186b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2958,7 +2958,8 @@ private:
 public:
   tree decl_container ();
   tree key_mergeable (int tag, merge_kind, tree decl, tree inner, tree type,
-		      tree container, bool is_attached);
+		      tree container, bool is_attached,
+		      bool is_imported_temploid_friend);
   unsigned binfo_mergeable (tree *);
 
 private:
@@ -7806,6 +7807,7 @@ trees_out::decl_value (tree decl, depset *dep)
 		       || !TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)));
 
   merge_kind mk = get_merge_kind (decl, dep);
+  bool is_imported_temploid_friend = imported_temploid_friends->get (decl);
 
   if (CHECKING_P)
     {
@@ -7841,13 +7843,11 @@ 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);
+
+	      /* Also tell the importer whether this is an imported temploid
+		 friend, which has implications for merging.  */
+	      bits.b (is_imported_temploid_friend);
 	    }
 	  bits.b (dep && dep->has_defn ());
 	}
@@ -8024,13 +8024,12 @@ trees_out::decl_value (tree decl, depset *dep)
 	}
     }
 
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      || TREE_CODE (inner) == TYPE_DECL)
+  if (is_imported_temploid_friend)
     {
       /* 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);
+      tree_node (*slot);
     }
 
   bool is_typedef = false;
@@ -8109,6 +8108,7 @@ trees_in::decl_value ()
 {
   int tag = 0;
   bool is_attached = false;
+  bool is_imported_temploid_friend = false;
   bool has_defn = false;
   unsigned mk_u = u ();
   if (mk_u >= MK_hwm || !merge_kind_name[mk_u])
@@ -8129,7 +8129,10 @@ trees_in::decl_value ()
 	{
 	  bits_in bits = stream_bits ();
 	  if (!(mk & MK_template_mask) && !state->is_header ())
-	    is_attached = bits.b ();
+	    {
+	      is_attached = bits.b ();
+	      is_imported_temploid_friend = bits.b ();
+	    }
 
 	  has_defn = bits.b ();
 	}
@@ -8234,7 +8237,7 @@ trees_in::decl_value ()
     parm_tag = fn_parms_init (inner);
 
   tree existing = key_mergeable (tag, mk, decl, inner, type, container,
-				 is_attached);
+				 is_attached, is_imported_temploid_friend);
   tree existing_inner = existing;
   if (existing)
     {
@@ -8339,8 +8342,7 @@ trees_in::decl_value ()
 	}
     }
 
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      || TREE_CODE (inner) == TYPE_DECL)
+  if (is_imported_temploid_friend)
     if (tree owner = tree_node ())
       if (is_new)
 	imported_temploid_friends->put (decl, owner);
@@ -11177,7 +11179,8 @@ check_mergeable_decl (merge_kind mk, tree decl, tree ovl, merge_key const &key)
 
 tree
 trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
-			 tree type, tree container, bool is_attached)
+			 tree type, tree container, bool is_attached,
+			 bool is_imported_temploid_friend)
 {
   const char *kind = "new";
   tree existing = NULL_TREE;
@@ -11319,6 +11322,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 
 	  case NAMESPACE_DECL:
 	    if (is_attached
+		&& !is_imported_temploid_friend
 		&& !(state->is_module () || state->is_partition ()))
 	      kind = "unique";
 	    else
@@ -11350,6 +11354,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 	    break;
 
 	  case TYPE_DECL:
+	    gcc_checking_assert (!is_imported_temploid_friend);
 	    if (is_attached && !(state->is_module () || state->is_partition ())
 		/* Implicit member functions can come from
 		   anywhere.  */
@@ -15356,6 +15361,7 @@ module_state::read_cluster (unsigned snum)
 	    tree visible = NULL_TREE;
 	    tree type = NULL_TREE;
 	    bool dedup = false;
+	    bool global_p = false;
 
 	    /* We rely on the bindings being in the reverse order of
 	       the resulting overload set.  */
@@ -15373,6 +15379,16 @@ module_state::read_cluster (unsigned snum)
 		if (sec.get_overrun ())
 		  break;
 
+		if (!global_p)
+		  {
+		    /* Check if the decl could require GM merging.  */
+		    tree orig = get_originating_module_decl (decl);
+		    tree inner = STRIP_TEMPLATE (orig);
+		    if (!DECL_LANG_SPECIFIC (inner)
+			|| !DECL_MODULE_ATTACH_P (inner))
+		      global_p = true;
+		  }
+
 		if (decls && TREE_CODE (decl) == TYPE_DECL)
 		  {
 		    /* Stat hack.  */
@@ -15459,10 +15475,8 @@ module_state::read_cluster (unsigned snum)
 	      break; /* Bail.  */
 
 	    dump () && dump ("Binding of %P", ns, name);
-	    if (!set_module_binding (ns, name, mod,
-				     is_header () ? -1
-				     : is_module () || is_partition () ? 1
-				     : 0,
+	    if (!set_module_binding (ns, name, mod, global_p,
+				     is_module () || is_partition (),
 				     decls, type, visible))
 	      sec.set_overrun ();
 	  }
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 8823ab71c60..872f1af0b2e 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -51,8 +51,8 @@ enum binding_slots
 {
  BINDING_SLOT_CURRENT,	/* Slot for current TU.  */
  BINDING_SLOT_GLOBAL,	/* Slot for merged global module. */
- BINDING_SLOT_PARTITION, /* Slot for merged partition entities
-			    (optional).  */
+ BINDING_SLOT_PARTITION, /* Slot for merged partition entities or
+			    imported friends.  */
 
  /* Number of always-allocated slots.  */
  BINDING_SLOTS_FIXED = BINDING_SLOT_GLOBAL + 1
@@ -248,9 +248,10 @@ get_fixed_binding_slot (tree *slot, tree name, unsigned ix, int create)
       if (!create)
 	return NULL;
 
-      /* The partition slot is only needed when we're a named
-	 module.  */
-      bool partition_slot = named_module_p ();
+      /* The partition slot is always needed, in case we have imported
+	 temploid friends with attachment different from the module we
+	 imported them from.  */
+      bool partition_slot = true;
       unsigned want = ((BINDING_SLOTS_FIXED + partition_slot + (create < 0)
 			+ BINDING_VECTOR_SLOTS_PER_CLUSTER - 1)
 		       / BINDING_VECTOR_SLOTS_PER_CLUSTER);
@@ -937,7 +938,6 @@ name_lookup::search_namespace_only (tree scope)
 		   stat_hack, then everything was exported.  */
 		tree type = NULL_TREE;
 
-
 		/* If STAT_HACK_P is false, everything is visible, and
 		   there's no duplication possibilities.  */
 		if (STAT_HACK_P (bind))
@@ -947,9 +947,9 @@ name_lookup::search_namespace_only (tree scope)
 			/* Do we need to engage deduplication?  */
 			int dup = 0;
 			if (MODULE_BINDING_GLOBAL_P (bind))
-			  dup = 1;
-			else if (MODULE_BINDING_PARTITION_P (bind))
-			  dup = 2;
+			  dup |= 1;
+			if (MODULE_BINDING_PARTITION_P (bind))
+			  dup |= 2;
 			if (unsigned hit = dup_detect & dup)
 			  {
 			    if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
@@ -1275,9 +1275,9 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
 			/* Do we need to engage deduplication?  */
 			int dup = 0;
 			if (MODULE_BINDING_GLOBAL_P (bind))
-			  dup = 1;
-			else if (MODULE_BINDING_PARTITION_P (bind))
-			  dup = 2;
+			  dup |= 1;
+			if (MODULE_BINDING_PARTITION_P (bind))
+			  dup |= 2;
 			if (unsigned hit = dup_detect & dup)
 			  if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
 			      || (hit & 2
@@ -3758,6 +3758,9 @@ check_module_override (tree decl, tree mvec, bool hiding,
   binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (mvec);
   unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (mvec);
 
+  tree nontmpl = STRIP_TEMPLATE (decl);
+  bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
+
   if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
     {
       cluster++;
@@ -3819,7 +3822,7 @@ check_module_override (tree decl, tree mvec, bool hiding,
     {
       /* Look in the appropriate mergeable decl slot.  */
       tree mergeable = NULL_TREE;
-      if (named_module_p ())
+      if (attached)
 	mergeable = BINDING_VECTOR_CLUSTER (mvec, BINDING_SLOT_PARTITION
 					   / BINDING_VECTOR_SLOTS_PER_CLUSTER)
 	  .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER];
@@ -3839,15 +3842,13 @@ check_module_override (tree decl, tree mvec, bool hiding,
  matched:
   if (match != error_mark_node)
     {
-      if (named_module_p ())
+      if (attached)
 	BINDING_VECTOR_PARTITION_DUPS_P (mvec) = true;
       else
 	BINDING_VECTOR_GLOBAL_DUPS_P (mvec) = true;
     }
 
   return match;
-
-  
 }
 
 /* Record DECL as belonging to the current lexical scope.  Check for
@@ -4300,7 +4301,9 @@ walk_module_binding (tree binding, bitmap partitions,
 	  cluster++;
 	}
 
-      bool maybe_dups = BINDING_VECTOR_PARTITION_DUPS_P (binding);
+      /* There could be duplicate module or GMF entries.  */
+      bool maybe_dups = (BINDING_VECTOR_PARTITION_DUPS_P (binding)
+			 || BINDING_VECTOR_GLOBAL_DUPS_P (binding));
 
       for (; ix--; cluster++)
 	for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
@@ -4394,14 +4397,14 @@ import_module_binding  (tree ns, tree name, unsigned mod, unsigned snum)
 }
 
 /* An import of MODULE is binding NS::NAME.  There should be no
-   existing binding for >= MODULE.  MOD_GLOB indicates whether MODULE
-   is a header_unit (-1) or part of the current module (+1).  VALUE
-   and TYPE are the value and type bindings. VISIBLE are the value
-   bindings being exported.  */
+   existing binding for >= MODULE.  GLOBAL_P indicates whether the
+   bindings include global module entities.  PARTITION_P is true if
+   it is part of the current module. VALUE and TYPE are the value
+   and type bindings. VISIBLE are the value bindings being exported.  */
 
 bool
-set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
-		    tree value, tree type, tree visible)
+set_module_binding (tree ns, tree name, unsigned mod, bool global_p,
+		    bool partition_p, tree value, tree type, tree visible)
 {
   if (!value)
     /* Bogus BMIs could give rise to nothing to bind.  */
@@ -4419,19 +4422,19 @@ set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
     return false;
 
   tree bind = value;
-  if (type || visible != bind || mod_glob)
+  if (type || visible != bind || partition_p || global_p)
     {
       bind = stat_hack (bind, type);
       STAT_VISIBLE (bind) = visible;
-      if ((mod_glob > 0 && TREE_PUBLIC (ns))
+      if ((partition_p && TREE_PUBLIC (ns))
 	  || (type && DECL_MODULE_EXPORT_P (type)))
 	STAT_TYPE_VISIBLE_P (bind) = true;
     }
 
-  /* Note if this is this-module or global binding.  */
-  if (mod_glob > 0)
+  /* Note if this is this-module and/or global binding.  */
+  if (partition_p)
     MODULE_BINDING_PARTITION_P (bind) = true;
-  else if (mod_glob < 0)
+  if (global_p)
     MODULE_BINDING_GLOBAL_P (bind) = true;
 
   *mslot = bind;
@@ -4540,10 +4543,8 @@ lookup_imported_hidden_friend (tree friend_tmpl)
       || !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);
+  tree bind = get_mergeable_namespace_binding
+    (current_namespace, DECL_NAME (inner), DECL_MODULE_ATTACH_P (inner));
   if (!bind)
     return NULL_TREE;
 
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 5cf6ae6374a..7c4193444dd 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -484,7 +484,7 @@ extern tree lookup_class_binding (tree ctx, tree name);
 extern bool import_module_binding (tree ctx, tree name, unsigned mod,
 				   unsigned snum);
 extern bool set_module_binding (tree ctx, tree name, unsigned mod,
-				int mod_glob_flag,
+				bool global_p, bool partition_p,
 				tree value, tree type, tree visible);
 extern void add_module_namespace_decl (tree ns, tree decl);
 
diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_a.C b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
new file mode 100644
index 00000000000..d5dcc93584a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
@@ -0,0 +1,7 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi A }
+
+export module A;
+
+extern "C++" int foo ();
+extern "C++" char bar ();
diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_b.C b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
new file mode 100644
index 00000000000..b94416aabbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi !B }
+
+export module B;
+import A;
+
+extern "C++" int foo ();
+extern "C++" int bar ();  // { dg-error "ambiguating new declaration" }
+
+// { dg-prune-output "not writing module" }
diff --git a/gcc/testsuite/g++.dg/modules/part-9_a.C b/gcc/testsuite/g++.dg/modules/part-9_a.C
new file mode 100644
index 00000000000..dc033d301ee
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_a.C
@@ -0,0 +1,10 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:a }
+
+module M:a;
+
+struct A {};
+extern "C++" struct B {};
+void f(int) {}
+extern "C++" void f(double) {}
diff --git a/gcc/testsuite/g++.dg/modules/part-9_b.C b/gcc/testsuite/g++.dg/modules/part-9_b.C
new file mode 100644
index 00000000000..7339da22d97
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_b.C
@@ -0,0 +1,10 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:b }
+
+module M:b;
+
+struct A {};
+extern "C++" struct B {};
+void f(int) {}
+extern "C++" void f(double) {}
diff --git a/gcc/testsuite/g++.dg/modules/part-9_c.C b/gcc/testsuite/g++.dg/modules/part-9_c.C
new file mode 100644
index 00000000000..26ac6777b7a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_c.C
@@ -0,0 +1,8 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+// Handle merging definitions of extern "C++" decls across partitions
+
+export module M;
+import :a;
+import :b;
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
index afbd0a39c23..b32fd98b756 100644
--- a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
@@ -1,8 +1,8 @@
 // { 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" }
+struct S {};  // { dg-message "previous declaration" "" { xfail *-*-* } }
+template <typename> struct T {};  // { dg-message "previous declaration" }
 void f() {}  // { dg-message "previously declared" }
 template <typename T> void g() {}  // { dg-message "previously declared" }
 
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15.h b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
new file mode 100644
index 00000000000..e4d3fff4445
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
@@ -0,0 +1,11 @@
+// PR c++/114950
+
+template <typename T>
+struct A {
+  friend void x();
+};
+template <typename T>
+struct B {
+  virtual void f() { A<T> r; }
+};
+template struct B<int>;
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
new file mode 100644
index 00000000000..04c800875f4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
@@ -0,0 +1,8 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:a }
+
+module M:a;
+extern "C++" {
+  #include "tpl-friend-15.h"
+}
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
new file mode 100644
index 00000000000..781882f97bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
@@ -0,0 +1,8 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:b }
+
+module M:b;
+extern "C++" {
+  #include "tpl-friend-15.h"
+}
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
new file mode 100644
index 00000000000..ced7e87d993
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
@@ -0,0 +1,7 @@
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+import :a;
+import :b;
diff mbox series

Patch

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1607a06757..e6b569ebca5 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2955,7 +2955,8 @@  private:
 public:
   tree decl_container ();
   tree key_mergeable (int tag, merge_kind, tree decl, tree inner, tree type,
-		      tree container, bool is_attached);
+		      tree container, bool is_attached,
+		      bool is_imported_temploid_friend);
   unsigned binfo_mergeable (tree *);
 
 private:
@@ -7803,6 +7804,7 @@  trees_out::decl_value (tree decl, depset *dep)
 		       || !TYPE_PTRMEMFUNC_P (TREE_TYPE (decl)));
 
   merge_kind mk = get_merge_kind (decl, dep);
+  bool is_imported_temploid_friend = imported_temploid_friends->get (decl);
 
   if (CHECKING_P)
     {
@@ -7838,13 +7840,11 @@  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);
+
+	      /* Also tell the importer whether this is an imported temploid
+		 friend, which has implications for merging.  */
+	      bits.b (is_imported_temploid_friend);
 	    }
 	  bits.b (dep && dep->has_defn ());
 	}
@@ -8021,13 +8021,12 @@  trees_out::decl_value (tree decl, depset *dep)
 	}
     }
 
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      || TREE_CODE (inner) == TYPE_DECL)
+  if (is_imported_temploid_friend)
     {
       /* 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);
+      tree_node (*slot);
     }
 
   bool is_typedef = false;
@@ -8106,6 +8105,7 @@  trees_in::decl_value ()
 {
   int tag = 0;
   bool is_attached = false;
+  bool is_imported_temploid_friend = false;
   bool has_defn = false;
   unsigned mk_u = u ();
   if (mk_u >= MK_hwm || !merge_kind_name[mk_u])
@@ -8126,7 +8126,10 @@  trees_in::decl_value ()
 	{
 	  bits_in bits = stream_bits ();
 	  if (!(mk & MK_template_mask) && !state->is_header ())
-	    is_attached = bits.b ();
+	    {
+	      is_attached = bits.b ();
+	      is_imported_temploid_friend = bits.b ();
+	    }
 
 	  has_defn = bits.b ();
 	}
@@ -8231,7 +8234,7 @@  trees_in::decl_value ()
     parm_tag = fn_parms_init (inner);
 
   tree existing = key_mergeable (tag, mk, decl, inner, type, container,
-				 is_attached);
+				 is_attached, is_imported_temploid_friend);
   tree existing_inner = existing;
   if (existing)
     {
@@ -8336,8 +8339,7 @@  trees_in::decl_value ()
 	}
     }
 
-  if (TREE_CODE (inner) == FUNCTION_DECL
-      || TREE_CODE (inner) == TYPE_DECL)
+  if (is_imported_temploid_friend)
     if (tree owner = tree_node ())
       if (is_new)
 	imported_temploid_friends->put (decl, owner);
@@ -11174,7 +11176,8 @@  check_mergeable_decl (merge_kind mk, tree decl, tree ovl, merge_key const &key)
 
 tree
 trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
-			 tree type, tree container, bool is_attached)
+			 tree type, tree container, bool is_attached,
+			 bool is_imported_temploid_friend)
 {
   const char *kind = "new";
   tree existing = NULL_TREE;
@@ -11316,6 +11319,7 @@  trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 
 	  case NAMESPACE_DECL:
 	    if (is_attached
+		&& !is_imported_temploid_friend
 		&& !(state->is_module () || state->is_partition ()))
 	      kind = "unique";
 	    else
@@ -11347,7 +11351,9 @@  trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
 	    break;
 
 	  case TYPE_DECL:
-	    if (is_attached && !(state->is_module () || state->is_partition ())
+	    if (is_attached
+		&& !is_imported_temploid_friend
+		&& !(state->is_module () || state->is_partition ())
 		/* Implicit member functions can come from
 		   anywhere.  */
 		&& !(DECL_ARTIFICIAL (decl)
@@ -15291,6 +15297,7 @@  module_state::read_cluster (unsigned snum)
 	    tree visible = NULL_TREE;
 	    tree type = NULL_TREE;
 	    bool dedup = false;
+	    bool global_p = false;
 
 	    /* We rely on the bindings being in the reverse order of
 	       the resulting overload set.  */
@@ -15308,6 +15315,16 @@  module_state::read_cluster (unsigned snum)
 		if (sec.get_overrun ())
 		  break;
 
+		if (!global_p)
+		  {
+		    /* Check if the decl could require GM merging.  */
+		    tree orig = get_originating_module_decl (decl);
+		    tree inner = STRIP_TEMPLATE (orig);
+		    if (!DECL_LANG_SPECIFIC (inner)
+			|| !DECL_MODULE_ATTACH_P (inner))
+		      global_p = true;
+		  }
+
 		if (decls && TREE_CODE (decl) == TYPE_DECL)
 		  {
 		    /* Stat hack.  */
@@ -15394,10 +15411,8 @@  module_state::read_cluster (unsigned snum)
 	      break; /* Bail.  */
 
 	    dump () && dump ("Binding of %P", ns, name);
-	    if (!set_module_binding (ns, name, mod,
-				     is_header () ? -1
-				     : is_module () || is_partition () ? 1
-				     : 0,
+	    if (!set_module_binding (ns, name, mod, global_p,
+				     is_module () || is_partition (),
 				     decls, type, visible))
 	      sec.set_overrun ();
 	  }
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 8823ab71c60..872f1af0b2e 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -51,8 +51,8 @@  enum binding_slots
 {
  BINDING_SLOT_CURRENT,	/* Slot for current TU.  */
  BINDING_SLOT_GLOBAL,	/* Slot for merged global module. */
- BINDING_SLOT_PARTITION, /* Slot for merged partition entities
-			    (optional).  */
+ BINDING_SLOT_PARTITION, /* Slot for merged partition entities or
+			    imported friends.  */
 
  /* Number of always-allocated slots.  */
  BINDING_SLOTS_FIXED = BINDING_SLOT_GLOBAL + 1
@@ -248,9 +248,10 @@  get_fixed_binding_slot (tree *slot, tree name, unsigned ix, int create)
       if (!create)
 	return NULL;
 
-      /* The partition slot is only needed when we're a named
-	 module.  */
-      bool partition_slot = named_module_p ();
+      /* The partition slot is always needed, in case we have imported
+	 temploid friends with attachment different from the module we
+	 imported them from.  */
+      bool partition_slot = true;
       unsigned want = ((BINDING_SLOTS_FIXED + partition_slot + (create < 0)
 			+ BINDING_VECTOR_SLOTS_PER_CLUSTER - 1)
 		       / BINDING_VECTOR_SLOTS_PER_CLUSTER);
@@ -937,7 +938,6 @@  name_lookup::search_namespace_only (tree scope)
 		   stat_hack, then everything was exported.  */
 		tree type = NULL_TREE;
 
-
 		/* If STAT_HACK_P is false, everything is visible, and
 		   there's no duplication possibilities.  */
 		if (STAT_HACK_P (bind))
@@ -947,9 +947,9 @@  name_lookup::search_namespace_only (tree scope)
 			/* Do we need to engage deduplication?  */
 			int dup = 0;
 			if (MODULE_BINDING_GLOBAL_P (bind))
-			  dup = 1;
-			else if (MODULE_BINDING_PARTITION_P (bind))
-			  dup = 2;
+			  dup |= 1;
+			if (MODULE_BINDING_PARTITION_P (bind))
+			  dup |= 2;
 			if (unsigned hit = dup_detect & dup)
 			  {
 			    if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
@@ -1275,9 +1275,9 @@  name_lookup::adl_namespace_fns (tree scope, bitmap imports)
 			/* Do we need to engage deduplication?  */
 			int dup = 0;
 			if (MODULE_BINDING_GLOBAL_P (bind))
-			  dup = 1;
-			else if (MODULE_BINDING_PARTITION_P (bind))
-			  dup = 2;
+			  dup |= 1;
+			if (MODULE_BINDING_PARTITION_P (bind))
+			  dup |= 2;
 			if (unsigned hit = dup_detect & dup)
 			  if ((hit & 1 && BINDING_VECTOR_GLOBAL_DUPS_P (val))
 			      || (hit & 2
@@ -3758,6 +3758,9 @@  check_module_override (tree decl, tree mvec, bool hiding,
   binding_cluster *cluster = BINDING_VECTOR_CLUSTER_BASE (mvec);
   unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (mvec);
 
+  tree nontmpl = STRIP_TEMPLATE (decl);
+  bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
+
   if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
     {
       cluster++;
@@ -3819,7 +3822,7 @@  check_module_override (tree decl, tree mvec, bool hiding,
     {
       /* Look in the appropriate mergeable decl slot.  */
       tree mergeable = NULL_TREE;
-      if (named_module_p ())
+      if (attached)
 	mergeable = BINDING_VECTOR_CLUSTER (mvec, BINDING_SLOT_PARTITION
 					   / BINDING_VECTOR_SLOTS_PER_CLUSTER)
 	  .slots[BINDING_SLOT_PARTITION % BINDING_VECTOR_SLOTS_PER_CLUSTER];
@@ -3839,15 +3842,13 @@  check_module_override (tree decl, tree mvec, bool hiding,
  matched:
   if (match != error_mark_node)
     {
-      if (named_module_p ())
+      if (attached)
 	BINDING_VECTOR_PARTITION_DUPS_P (mvec) = true;
       else
 	BINDING_VECTOR_GLOBAL_DUPS_P (mvec) = true;
     }
 
   return match;
-
-  
 }
 
 /* Record DECL as belonging to the current lexical scope.  Check for
@@ -4300,7 +4301,9 @@  walk_module_binding (tree binding, bitmap partitions,
 	  cluster++;
 	}
 
-      bool maybe_dups = BINDING_VECTOR_PARTITION_DUPS_P (binding);
+      /* There could be duplicate module or GMF entries.  */
+      bool maybe_dups = (BINDING_VECTOR_PARTITION_DUPS_P (binding)
+			 || BINDING_VECTOR_GLOBAL_DUPS_P (binding));
 
       for (; ix--; cluster++)
 	for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
@@ -4394,14 +4397,14 @@  import_module_binding  (tree ns, tree name, unsigned mod, unsigned snum)
 }
 
 /* An import of MODULE is binding NS::NAME.  There should be no
-   existing binding for >= MODULE.  MOD_GLOB indicates whether MODULE
-   is a header_unit (-1) or part of the current module (+1).  VALUE
-   and TYPE are the value and type bindings. VISIBLE are the value
-   bindings being exported.  */
+   existing binding for >= MODULE.  GLOBAL_P indicates whether the
+   bindings include global module entities.  PARTITION_P is true if
+   it is part of the current module. VALUE and TYPE are the value
+   and type bindings. VISIBLE are the value bindings being exported.  */
 
 bool
-set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
-		    tree value, tree type, tree visible)
+set_module_binding (tree ns, tree name, unsigned mod, bool global_p,
+		    bool partition_p, tree value, tree type, tree visible)
 {
   if (!value)
     /* Bogus BMIs could give rise to nothing to bind.  */
@@ -4419,19 +4422,19 @@  set_module_binding (tree ns, tree name, unsigned mod, int mod_glob,
     return false;
 
   tree bind = value;
-  if (type || visible != bind || mod_glob)
+  if (type || visible != bind || partition_p || global_p)
     {
       bind = stat_hack (bind, type);
       STAT_VISIBLE (bind) = visible;
-      if ((mod_glob > 0 && TREE_PUBLIC (ns))
+      if ((partition_p && TREE_PUBLIC (ns))
 	  || (type && DECL_MODULE_EXPORT_P (type)))
 	STAT_TYPE_VISIBLE_P (bind) = true;
     }
 
-  /* Note if this is this-module or global binding.  */
-  if (mod_glob > 0)
+  /* Note if this is this-module and/or global binding.  */
+  if (partition_p)
     MODULE_BINDING_PARTITION_P (bind) = true;
-  else if (mod_glob < 0)
+  if (global_p)
     MODULE_BINDING_GLOBAL_P (bind) = true;
 
   *mslot = bind;
@@ -4540,10 +4543,8 @@  lookup_imported_hidden_friend (tree friend_tmpl)
       || !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);
+  tree bind = get_mergeable_namespace_binding
+    (current_namespace, DECL_NAME (inner), DECL_MODULE_ATTACH_P (inner));
   if (!bind)
     return NULL_TREE;
 
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 5cf6ae6374a..7c4193444dd 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -484,7 +484,7 @@  extern tree lookup_class_binding (tree ctx, tree name);
 extern bool import_module_binding (tree ctx, tree name, unsigned mod,
 				   unsigned snum);
 extern bool set_module_binding (tree ctx, tree name, unsigned mod,
-				int mod_glob_flag,
+				bool global_p, bool partition_p,
 				tree value, tree type, tree visible);
 extern void add_module_namespace_decl (tree ns, tree decl);
 
diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_a.C b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
new file mode 100644
index 00000000000..d5dcc93584a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/ambig-2_a.C
@@ -0,0 +1,7 @@ 
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi A }
+
+export module A;
+
+extern "C++" int foo ();
+extern "C++" char bar ();
diff --git a/gcc/testsuite/g++.dg/modules/ambig-2_b.C b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
new file mode 100644
index 00000000000..b94416aabbf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/ambig-2_b.C
@@ -0,0 +1,10 @@ 
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi !B }
+
+export module B;
+import A;
+
+extern "C++" int foo ();
+extern "C++" int bar ();  // { dg-error "ambiguating new declaration" }
+
+// { dg-prune-output "not writing module" }
diff --git a/gcc/testsuite/g++.dg/modules/part-9_a.C b/gcc/testsuite/g++.dg/modules/part-9_a.C
new file mode 100644
index 00000000000..dc033d301ee
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_a.C
@@ -0,0 +1,10 @@ 
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:a }
+
+module M:a;
+
+struct A {};
+extern "C++" struct B {};
+void f(int) {}
+extern "C++" void f(double) {}
diff --git a/gcc/testsuite/g++.dg/modules/part-9_b.C b/gcc/testsuite/g++.dg/modules/part-9_b.C
new file mode 100644
index 00000000000..7339da22d97
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_b.C
@@ -0,0 +1,10 @@ 
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:b }
+
+module M:b;
+
+struct A {};
+extern "C++" struct B {};
+void f(int) {}
+extern "C++" void f(double) {}
diff --git a/gcc/testsuite/g++.dg/modules/part-9_c.C b/gcc/testsuite/g++.dg/modules/part-9_c.C
new file mode 100644
index 00000000000..26ac6777b7a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/part-9_c.C
@@ -0,0 +1,8 @@ 
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+// Handle merging definitions of extern "C++" decls across partitions
+
+export module M;
+import :a;
+import :b;
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
index afbd0a39c23..b32fd98b756 100644
--- a/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-13_e.C
@@ -1,8 +1,8 @@ 
 // { 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" }
+struct S {};  // { dg-message "previous declaration" "" { xfail *-*-* } }
+template <typename> struct T {};  // { dg-message "previous declaration" }
 void f() {}  // { dg-message "previously declared" }
 template <typename T> void g() {}  // { dg-message "previously declared" }
 
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15.h b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
new file mode 100644
index 00000000000..e4d3fff4445
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15.h
@@ -0,0 +1,11 @@ 
+// PR c++/114950
+
+template <typename T>
+struct A {
+  friend void x();
+};
+template <typename T>
+struct B {
+  virtual void f() { A<T> r; }
+};
+template struct B<int>;
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
new file mode 100644
index 00000000000..04c800875f4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_a.C
@@ -0,0 +1,8 @@ 
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:a }
+
+module M:a;
+extern "C++" {
+  #include "tpl-friend-15.h"
+}
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
new file mode 100644
index 00000000000..781882f97bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_b.C
@@ -0,0 +1,8 @@ 
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M:b }
+
+module M:b;
+extern "C++" {
+  #include "tpl-friend-15.h"
+}
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
new file mode 100644
index 00000000000..ced7e87d993
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-15_c.C
@@ -0,0 +1,7 @@ 
+// PR c++/114950
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi M }
+
+export module M;
+import :a;
+import :b;