On Sat, Mar 29, 2025 at 11:14:46AM -0400, Jason Merrill wrote:
> Tested x86_64-pc-linux-gnu, applying to trunk. Do you agree with my choice of
> how to adjust duplicate_decls?
Yes, that looks reasonable to me. The point of this block is just to
ensure that we export any entities declared in the module, but an
injected friend isn't really a declaration in the same way. I also
can't think of any other cases this might come up.
(Similarly, the fact that the early check for 'DECL_LANG_SPECIFIC'
breaks propagation of "not imported" for non-lang-specific decls doesn't
actually affect anything I can think of, because the only time this
would be relevant is if the new declaration was in module purview in
which case it would have a lang-specific.)
>
> -- 8< --
>
> Here we were failing to match the injected friend declaration to the
> definition because the latter isn't exported. But the friend is attached to
> the module, so we need to look for any reachable declaration in that module,
> not just the exports.
>
> The duplicate_decls change is to avoid clobbering DECL_MODULE_IMPORT_P on
> the imported definition; matching an injected friend doesn't change that
> it's imported. I considered checking get_originating_module == 0 or
> !decl_defined_p instead of DECL_UNIQUE_FRIEND_P there, but I think this
> situation is specific to friends.
>
> I removed an assert because we have a test for the same condition a few
> lines above.
>
> gcc/cp/ChangeLog:
>
> * decl.cc (duplicate_decls): Don't clobber DECL_MODULE_IMPORT_P with
> an injected friend.
> * name-lookup.cc (check_module_override): Look at all reachable
> decls in decl's originating module.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/friend-9_a.C: New test.
> * g++.dg/modules/friend-9_b.C: New test.
> ---
> gcc/cp/decl.cc | 6 ++++--
> gcc/cp/name-lookup.cc | 19 ++++++++++---------
> gcc/testsuite/g++.dg/modules/friend-9_a.C | 13 +++++++++++++
> gcc/testsuite/g++.dg/modules/friend-9_b.C | 13 +++++++++++++
> 4 files changed, 40 insertions(+), 11 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/friend-9_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/friend-9_b.C
>
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index a785d5e79cb..7d10b228ec6 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -2539,8 +2539,10 @@ duplicate_decls (tree newdecl, tree olddecl, bool
> hiding, bool was_hidden)
> }
>
> /* Propagate purviewness and importingness as with
> - set_instantiating_module. */
> - if (modules_p () && DECL_LANG_SPECIFIC (new_result))
> + set_instantiating_module, unless newdecl is a friend injection. */
> + if (modules_p () && DECL_LANG_SPECIFIC (new_result)
> + && !(TREE_CODE (new_result) == FUNCTION_DECL
> + && DECL_UNIQUE_FRIEND_P (new_result)))
> {
> if (DECL_MODULE_PURVIEW_P (new_result))
> DECL_MODULE_PURVIEW_P (old_result) = true;
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index df033edafc7..7fadbccfe39 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -3777,6 +3777,10 @@ check_module_override (tree decl, tree mvec, bool
> hiding,
> any reachable declaration, so we should check for overriding here too.
> */
> bool any_reachable = deduction_guide_p (decl);
>
> + /* DECL might have an originating module if it's an instantiation of a
> + friend; we want to look at all reachable decls in that module. */
> + unsigned decl_mod = get_originating_module (decl);
> +
> if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
> {
> cluster++;
> @@ -3789,18 +3793,15 @@ check_module_override (tree decl, tree mvec, bool
> hiding,
> /* Are we importing this module? */
> if (cluster->indices[jx].span != 1)
> continue;
> - if (!cluster->indices[jx].base)
> + unsigned cluster_mod = cluster->indices[jx].base;
> + if (!cluster_mod)
> continue;
> - if (!any_reachable
> - && !bitmap_bit_p (imports, cluster->indices[jx].base))
> + bool c_any_reachable = (any_reachable || cluster_mod == decl_mod);
> + if (!c_any_reachable && !bitmap_bit_p (imports, cluster_mod))
> continue;
> /* Is it loaded? */
> if (cluster->slots[jx].is_lazy ())
> - {
> - gcc_assert (cluster->indices[jx].span == 1);
> - lazy_load_binding (cluster->indices[jx].base,
> - scope, name, &cluster->slots[jx]);
> - }
> + lazy_load_binding (cluster_mod, scope, name, &cluster->slots[jx]);
> tree bind = cluster->slots[jx];
> if (!bind)
> /* Errors could cause there to be nothing. */
> @@ -3812,7 +3813,7 @@ check_module_override (tree decl, tree mvec, bool
> hiding,
> /* If there was a matching STAT_TYPE here then xref_tag
> should have found it, but we need to check anyway because
> a conflicting using-declaration may exist. */
> - if (any_reachable)
> + if (c_any_reachable)
> {
> type = STAT_TYPE (bind);
> bind = STAT_DECL (bind);
> diff --git a/gcc/testsuite/g++.dg/modules/friend-9_a.C
> b/gcc/testsuite/g++.dg/modules/friend-9_a.C
> new file mode 100644
> index 00000000000..ca95027b470
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/friend-9_a.C
> @@ -0,0 +1,13 @@
> +// { dg-additional-options -fmodules }
> +// { dg-module-cmi M }
> +// { dg-module-do link }
> +
> +export module M;
> +
> +export template <class T> struct A
> +{
> + template <class U> friend void f (U);
> +};
> +
> +template <class U>
> +void f(U u) { }
> diff --git a/gcc/testsuite/g++.dg/modules/friend-9_b.C
> b/gcc/testsuite/g++.dg/modules/friend-9_b.C
> new file mode 100644
> index 00000000000..9f583795099
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/friend-9_b.C
> @@ -0,0 +1,13 @@
> +// { dg-additional-options -fmodules }
> +
> +// Check that f and A are mangled as attached to M.
> +// void f@M<A@M<main::loc> >(A@M<main::loc>)
> +// { dg-final { scan-assembler "_ZW1M1fIS_1AIZ4mainE3locEEvT_" } }
> +
> +import M;
> +
> +int main()
> +{
> + struct loc {};
> + f(A<loc>());
> +}
>
> base-commit: 5ac4be28822a4e6f506a69096f92d4675a7d5a72
> --
> 2.49.0
>
