c++/modules: Fix ADL [PR117658]

On looking again at [basic.lookup.argdep] p4, I believe GCC hasn't fully
implemented the wording here for ADL.  This patch fixes two issues.

First, 4.3 indicates that a function exported from a named module should
be visible to ADL regardless of whether it's visible to normal name
lookup, as long as some restrictions are followed.

This patch implements this; for skipping declarations that "do not
appear in the TU containing the point of lookup" I don't think there's
anything special we need to do, as any declarations before the point of
lookup will be found in other ways anyway, and any remaining
declarations from the current TU cannot be seen regardless.

Secondly, currently we only add the exported functions along the
instantiation path of a lookup.  But I don't think this is intended by
the current wording, so this patch adjusts that.  I also clean up the
logic to do all different module processing in adl_namespace_fns so that
we don't duplicate work in traversing the module binding list
unnecessarily.

This new handling means we need to do some extra work to properly error
on overload sets containing TU-local entities (as this might actually
come up now!) but I'm leaving that for a later patch.

As a drive-by fix this also fixes an ICE for C++26 expansion statements
with finding the instantiation path.

	PR c++/117658

gcc/cp/ChangeLog:

	* cp-tree.h (get_originating_module): Adjust parameter names.
	* module.cc (path_of_instantiation): Handle C++26 expansion
	statements.
	* name-lookup.cc (name_lookup::adl_namespace_fns): Handle
	exported declarations attached to the same module of an
	associated entity with the same innermost non-inline namespace,
	and non-exported functions on the instantiation path.
	(name_lookup::search_adl): Build mapping of namespace to modules
	that associated entities are attached to; remove now-unneeded
	instantiation path handling.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/adl-4_a.C: Test should pass.
	* g++.dg/modules/adl-4_b.C: Test should pass.
	* g++.dg/modules/adl-6_a.C: New test.
	* g++.dg/modules/adl-6_b.C: New test.
	* g++.dg/modules/adl-6_c.C: New test.
	* g++.dg/modules/adl-7_a.C: New test.
	* g++.dg/modules/adl-7_b.C: New test.
	* g++.dg/modules/adl-7_c.C: New test.
	* g++.dg/modules/adl-8_a.C: New test.
	* g++.dg/modules/adl-8_b.C: New test.
	* g++.dg/modules/adl-8_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8520ca05556c8f01e18c573f7699b8d97a768c67..d531bcd833b3028213242bc534fb142718c944fc 100644 (file)
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7745,7 +7745,7 @@ extern void module_add_import_initializers ();
 /* Where the namespace-scope decl was originally declared.  */
 extern void set_originating_module (tree, bool friend_p = false);
 extern tree get_originating_module_decl (tree) ATTRIBUTE_PURE;
-extern int get_originating_module (tree, bool for_mangle = false) ATTRIBUTE_PURE;
+extern int get_originating_module (tree, bool global_m1 = false) ATTRIBUTE_PURE;
 extern unsigned get_importing_module (tree, bool = false) ATTRIBUTE_PURE;
 extern void check_module_decl_linkage (tree);
 

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 0404eae6ce3652bbd46690833f4d529fca7929ed..ee76dd863d71c14ec0156f584d74c2c73af9a660 100644 (file)
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -20833,8 +20833,9 @@ path_of_instantiation (tinst_level *tinst,  bitmap *path_map_p)
 {
   gcc_checking_assert (modules_p ());
 
-  if (!tinst)
+  if (!tinst || TREE_CODE (tinst->tldcl) == TEMPLATE_FOR_STMT)
     {
+      gcc_assert (!tinst || !tinst->next);
       /* Not inside an instantiation, just the regular case.  */
       *path_map_p = nullptr;
       return get_import_bitmap ();

diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index f5e80503678e3f1a5ef48062611f901ba2a2daea..9a2d0747ba48b6064f2181d26d3168b7c6898616 100644 (file)
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -550,7 +550,7 @@ private:
   void adl_class_only (tree);
   void adl_namespace (tree);
   void adl_class_fns (tree);
-  void adl_namespace_fns (tree, bitmap);
+  void adl_namespace_fns (tree, bitmap, bitmap, bitmap);
 
 public:
   /* Search namespace + inlines + maybe usings as qualified lookup.  */
@@ -1205,10 +1205,16 @@ name_lookup::add_fns (tree fns)
   add_overload (fns);
 }
 
-/* Add the overloaded fns of SCOPE.  */
+/* Add the overloaded fns of SCOPE.  IMPORTS is the list of visible modules
+   for this lookup. INST_PATH for dependent (2nd phase) ADL is the list of
+   modules on the instantiation context for this lookup, or otherwise NULL.
+   ASSOCS is the list of modules where this namespace shares an innermost
+   non-inline namespace with an associated entity attached to said module,
+   or NULL if there are none.  */
 
 void
-name_lookup::adl_namespace_fns (tree scope, bitmap imports)
+name_lookup::adl_namespace_fns (tree scope, bitmap imports,
+                               bitmap inst_path, bitmap assocs)
 {
   if (tree *binding = find_namespace_slot (scope, name))
     {
@@ -1257,19 +1263,22 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
          for (; ix--; cluster++)
            for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
              {
+               int mod = cluster->indices[jx].base;
+
                /* Functions are never on merged slots.  */
-               if (!cluster->indices[jx].base
-                   || cluster->indices[jx].span != 1)
+               if (!mod || cluster->indices[jx].span != 1)
                  continue;
 
-               /* Is this slot visible?  */
-               if (!bitmap_bit_p (imports, cluster->indices[jx].base))
+               /* Is this slot accessible here?  */
+               bool visible = bitmap_bit_p (imports, mod);
+               bool on_inst_path = inst_path && bitmap_bit_p (inst_path, mod);
+               if (!visible && !on_inst_path
+                   && !(assocs && bitmap_bit_p (assocs, mod)))
                  continue;
 
-               /* Is it loaded.  */
+               /* Is it loaded?  */
                if (cluster->slots[jx].is_lazy ())
-                 lazy_load_binding (cluster->indices[jx].base,
-                                    scope, name, &cluster->slots[jx]);
+                 lazy_load_binding (mod, scope, name, &cluster->slots[jx]);
 
                tree bind = cluster->slots[jx];
                if (!bind)
@@ -1294,10 +1303,26 @@ name_lookup::adl_namespace_fns (tree scope, bitmap imports)
                        dup_detect |= dup;
                      }
 
-                   bind = STAT_VISIBLE (bind);
+                   /* For lookups on the instantiation path we can see any
+                      module-linkage declaration; otherwise we should only
+                      see exported decls.  */
+                   if (!on_inst_path)
+                     bind = STAT_VISIBLE (bind);
                  }
 
-               add_fns (bind);
+               if (on_inst_path || visible)
+                 add_fns (bind);
+               else
+                 {
+                   /* We're only accessible because we're the same module as
+                      an associated entity with module attachment: only add
+                      functions actually attached to this module.  */
+                   for (tree fn : ovl_range (bind))
+                     if (DECL_DECLARES_FUNCTION_P (fn)
+                         && DECL_LANG_SPECIFIC (STRIP_TEMPLATE (fn))
+                         && DECL_MODULE_ATTACH_P (STRIP_TEMPLATE (fn)))
+                       add_overload (fn);
+                 }
              }
        }
     }
@@ -1632,6 +1657,32 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc> *args)
       if (fns)
        dedup (true);
 
+      /* First get the attached modules for each innermost non-inline
+        namespace of an associated entity.  */
+      bitmap_obstack_initialize (NULL);
+      hash_map<tree, bitmap> ns_mod_assocs;
+      if (modules_p ())
+       {
+         for (tree scope : scopes)
+           if (TYPE_P (scope))
+             {
+               int mod = get_originating_module (TYPE_NAME (scope),
+                                                 /*global_m1=*/true);
+               if (mod > 0)
+                 {
+                   tree ctx = decl_namespace_context (scope);
+                   while (DECL_NAMESPACE_INLINE_P (ctx))
+                     ctx = CP_DECL_CONTEXT (ctx);
+
+                   bool existed = false;
+                   bitmap &b = ns_mod_assocs.get_or_insert (ctx, &existed);
+                   if (!existed)
+                     b = BITMAP_ALLOC (NULL);
+                   bitmap_set_bit (b, mod);
+                 }
+             }
+       }
+
       /* INST_PATH will be NULL, if this is /not/ 2nd-phase ADL.  */
       bitmap inst_path = NULL;
       /* VISIBLE is the regular import bitmap.  */
@@ -1641,66 +1692,22 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc> *args)
        {
          tree scope = (*scopes)[ix];
          if (TREE_CODE (scope) == NAMESPACE_DECL)
-           adl_namespace_fns (scope, visible);
-         else
            {
-             if (RECORD_OR_UNION_TYPE_P (scope))
-               adl_class_fns (scope);
-
-             /* During 2nd phase ADL: Any exported declaration D in N
-                declared within the purview of a named module M
-                (10.2) is visible if there is an associated entity
-                attached to M with the same innermost enclosing
-                non-inline namespace as D.
-                [basic.lookup.argdep]/4.4 */
-
-             if (!inst_path)
-               /* Not 2nd phase.  */
-               continue;
-
-             tree ctx = CP_DECL_CONTEXT (TYPE_NAME (scope));
-             if (TREE_CODE (ctx) != NAMESPACE_DECL)
-               /* Not namespace-scope class.  */
-               continue;
-
-             tree origin = get_originating_module_decl (TYPE_NAME (scope));
-             tree not_tmpl = STRIP_TEMPLATE (origin);
-             if (!DECL_LANG_SPECIFIC (not_tmpl)
-                 || !DECL_MODULE_IMPORT_P (not_tmpl))
-               /* Not imported.  */
-               continue;
-
-             unsigned module = get_importing_module (origin);
-
-             if (!bitmap_bit_p (inst_path, module))
-               /* Not on path of instantiation.  */
-               continue;
-
-             if (bitmap_bit_p (visible, module))
-               /* If the module was in the visible set, we'll look at
-                  its namespace partition anyway.  */
-               continue;
-
-             if (tree *slot = find_namespace_slot (ctx, name, false))
-               if (binding_slot *mslot = search_imported_binding_slot (slot, module))
-                 {
-                   if (mslot->is_lazy ())
-                     lazy_load_binding (module, ctx, name, mslot);
-
-                   if (tree bind = *mslot)
-                     {
-                       /* We must turn on deduping, because some other class
-                          from this module might also be in this namespace.  */
-                       dedup (true);
-
-                       /* Add the exported fns  */
-                       if (STAT_HACK_P (bind))
-                         add_fns (STAT_VISIBLE (bind));
-                     }
-                 }
+             tree ctx = scope;
+             while (DECL_NAMESPACE_INLINE_P (ctx))
+               ctx = CP_DECL_CONTEXT (ctx);
+             bitmap *assocs = ns_mod_assocs.get (ctx);
+             adl_namespace_fns (scope, visible, inst_path,
+                                assocs ? *assocs : NULL);
            }
+         else if (RECORD_OR_UNION_TYPE_P (scope))
+           adl_class_fns (scope);
        }
 
+      for (auto refs : ns_mod_assocs)
+       BITMAP_FREE (refs.second);
+      bitmap_obstack_release (NULL);
+
       fns = value;
       dedup (false);
     }

diff --git a/gcc/testsuite/g++.dg/modules/adl-4_a.C b/gcc/testsuite/g++.dg/modules/adl-4_a.C
index 5d956c057e2018f1160d59d967e3d62b0dfde185..f00ee55e1640c06ae5fe9b4e79f636362e8ea7c4 100644 (file)
--- a/gcc/testsuite/g++.dg/modules/adl-4_a.C
+++ b/gcc/testsuite/g++.dg/modules/adl-4_a.C
@@ -3,7 +3,7 @@ export module inter;
 // { dg-module-cmi inter }
 
 namespace hidden {
-// not found via ADL
+
 int fn (int x);
 
 }

diff --git a/gcc/testsuite/g++.dg/modules/adl-4_b.C b/gcc/testsuite/g++.dg/modules/adl-4_b.C
index aa1396f7ce2b8227f90e2f80b7f9b9d1cb154ca9..91c1d8a20478a8b6f2877995df9d127a30719122 100644 (file)
--- a/gcc/testsuite/g++.dg/modules/adl-4_b.C
+++ b/gcc/testsuite/g++.dg/modules/adl-4_b.C
@@ -25,12 +25,11 @@ int main ()
 {
   hidden::Y y(2);
 
-  // unexported hidden::fn@inter is not visible from TPL@inter
+  // unexported hidden::fn@inter is visible from TPL@inter because it's a
+  // dependent name and so lookup is performed at each point on the
+  // instantiation context, including the point at the end of inter.
   if (TPL (y) != -2)
     return 2;
 
   return 0;
 }
-
-// ADL fails
-// { dg-regexp {[^\n]*/adl-4_a.C:14:[0-9]*: error: 'fn' was not declared in this scope\n} }

diff --git a/gcc/testsuite/g++.dg/modules/adl-6_a.C b/gcc/testsuite/g++.dg/modules/adl-6_a.C
new file mode 100644 (file)
index 0000000..e978903
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-6_a.C
@@ -0,0 +1,38 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi M }
+
+export module M;
+
+namespace R {
+  export struct X {};
+  export void f(X);
+}
+
+namespace S {
+  export void f(R::X, R::X);
+}
+
+namespace I {
+  inline namespace A {
+    export struct Y {};
+  }
+  inline namespace B {
+    export void f(Y);
+    export extern "C++" void f(Y, int);
+  }
+  inline namespace C {
+    export struct f {} f;
+  }
+}
+
+namespace O {
+  export void g(I::Y);
+  export extern "C++" void h(I::Y);
+}
+namespace I {
+  inline namespace D {
+    export using O::g;
+    export using O::h;
+  }
+}

diff --git a/gcc/testsuite/g++.dg/modules/adl-6_b.C b/gcc/testsuite/g++.dg/modules/adl-6_b.C
new file mode 100644 (file)
index 0000000..a373b4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-6_b.C
@@ -0,0 +1,26 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi N }
+
+export module N;
+import M;
+
+export R::X make();
+
+namespace R {
+  static int g(X);
+}
+
+export
+template<typename T, typename U>
+void apply_ok(T t, U u) {
+  f(t, u);
+}
+
+export
+template<typename T>
+void apply_err(T t) {
+  g(t);
+}
+
+export I::Y make_Y();

diff --git a/gcc/testsuite/g++.dg/modules/adl-6_c.C b/gcc/testsuite/g++.dg/modules/adl-6_c.C
new file mode 100644 (file)
index 0000000..99b6c4c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-6_c.C
@@ -0,0 +1,36 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+
+import N;
+
+namespace S {
+  struct Z { template<typename T> operator T(); };
+}
+
+void test() {
+  auto x = make();  // OK, decltype(x) is R::X in module M
+
+  R::f(x);  // error: R and R::f are not visible here
+  // { dg-error "" "" { target *-*-* } .-1 }
+
+  f(x);  // OK, calls R::f from interface of M
+
+  f(x, S::Z());  // error: S::f in module M not considered even though S is an associated namespace
+  // { dg-error "" "" { target *-*-* } .-1 }
+
+  apply_ok(x, S::Z());  // OK, S::f is visible in instantiation context
+
+  apply_err(x);  // error: R::g has internal linkage and cannot be used outside N
+  // { dg-message "here" "" { target *-*-* } .-1 }
+  // { dg-error "'g'" "" { target *-*-* } 0 }
+
+  auto y = make_Y();
+  f(y);  // OK, I::B::f and I::A::Y have matching innermost non-inline namespace
+  g(y);  // OK, O::g is accessible through I::D::g
+
+  f(y, 0);  // error: I::B::f(Y, int) is not attached to M
+  // { dg-error "" "" { target *-*-* } .-1 }
+
+  h(y);  // error: O::h is also not attached to M
+  // { dg-error "" "" { target *-*-* } .-1 }
+}

diff --git a/gcc/testsuite/g++.dg/modules/adl-7_a.C b/gcc/testsuite/g++.dg/modules/adl-7_a.C
new file mode 100644 (file)
index 0000000..50c2432
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_a.C
@@ -0,0 +1,18 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi A }
+
+export module A;
+
+export template <typename T> void test(T t) { foo(t); }
+
+namespace ns1 {
+  export struct S {};
+  /* not exported */ void foo(S) {}
+}
+
+namespace ns2 {
+  export template <typename T> struct S {
+    friend void foo(S) {}
+  };
+}

diff --git a/gcc/testsuite/g++.dg/modules/adl-7_b.C b/gcc/testsuite/g++.dg/modules/adl-7_b.C
new file mode 100644 (file)
index 0000000..5c615c0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_b.C
@@ -0,0 +1,8 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi B }
+
+export module B;
+export import A;
+
+export using bar = ns2::S<int>;

diff --git a/gcc/testsuite/g++.dg/modules/adl-7_c.C b/gcc/testsuite/g++.dg/modules/adl-7_c.C
new file mode 100644 (file)
index 0000000..18d3915
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_c.C
@@ -0,0 +1,9 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+
+import B;
+
+int main() {
+  ::test(ns1::S{});
+  ::test(bar{});
+}

diff --git a/gcc/testsuite/g++.dg/modules/adl-8_a.C b/gcc/testsuite/g++.dg/modules/adl-8_a.C
new file mode 100644 (file)
index 0000000..800d9f8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_a.C
@@ -0,0 +1,23 @@
+// C++26 expansion statements should not ICE
+// { dg-additional-options "-fmodules -std=c++26" }
+// { dg-module-cmi M }
+
+export module M;
+
+namespace ns {
+  export struct S {};
+  void foo(S) {}
+}
+
+export void test1() {
+  template for (auto x : { ns::S{} }) {
+    foo(x);
+  }
+}
+
+export template <typename T>
+void test2(T t) {
+  template for (auto x : { t, t, t }) {
+    foo(x);
+  }
+}

diff --git a/gcc/testsuite/g++.dg/modules/adl-8_b.C b/gcc/testsuite/g++.dg/modules/adl-8_b.C
new file mode 100644 (file)
index 0000000..77049a2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_b.C
@@ -0,0 +1,14 @@
+// { dg-additional-options "-fmodules -std=c++26" }
+
+export module X;
+export import M;
+
+export template <typename T>
+void test3(T t) {
+  test2(t);
+}
+
+namespace other {
+  export struct S {};
+  void foo(S) {}
+}

diff --git a/gcc/testsuite/g++.dg/modules/adl-8_c.C b/gcc/testsuite/g++.dg/modules/adl-8_c.C
new file mode 100644 (file)
index 0000000..0d64ac3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_c.C
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodules -std=c++26" }
+
+import X;
+
+int main() {
+  test1();
+  test2(ns::S{});
+  test3(other::S{});
+}