[clang] 84bf4ab - [C++20] [Modules] Allow ADL in dependent context for modules

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-02-07T14:09:46+08:00
New Revision: 84bf4ab087b105e0c72da9ec26ad45e9468fa7be

URL: https://github.com/llvm/llvm-project/commit/84bf4ab087b105e0c72da9ec26ad45e9468fa7be
DIFF: https://github.com/llvm/llvm-project/commit/84bf4ab087b105e0c72da9ec26ad45e9468fa7be.diff

LOG: [C++20] [Modules] Allow ADL in dependent context for modules

Close https://github.com/llvm/llvm-project/issues/60488.

Previously, when we instantiate a template, the argument dependent
lookup is performed in the context of the instantiation, which implies that the
functions not visible in the context can't be found by the argument
dependent lookup.

But this is not true, according to [module.context]p3, the instantiation
context for the implicit instantiation of a template should contain the
context of the primary module interface if the template is defined in
the module interface unit.

Note that the fix didn't implemnet [module.context]p3 precisely, see the
comments for example.

Added: 
    clang/test/Modules/named-modules-adl-2.cppm
    clang/test/Modules/named-modules-adl.cppm

Modified: 
    clang/lib/Sema/SemaLookup.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index b2e943699c5f1..b910974d2ce61 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3883,10 +3883,46 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
           if (isVisible(D)) {
             Visible = true;
             break;
-          } else if (getLangOpts().CPlusPlusModules &&
-                     D->isInExportDeclContext()) {
+          }
+
+          if (!getLangOpts().CPlusPlusModules)
+            continue;
+
+          // A partial mock implementation for instantiation context for
+          // modules. [module.context]p1:
+          //    The instantiation context is a set of points within the program
+          //    that determines which declarations are found by
+          //    argument-dependent name lookup...
+          //
+          // FIXME: This didn't implement [module.context] well. For example,
+          // [module.context]p3 says:
+          //    During the implicit instantiation of a template whose point of
+          //    instantiation is specified as that of an enclosing
+          //    specialization if the template is defined in a module interface
+          //    unit of a module M and the point of instantiation is not in a
+          //    module interface unit of M, the point at the end of the
+          //    declaration-seq of the primary module interface unit of M.
+          //
+          // But we didn't check if the template is defined in a module
+          // interface unit nor we check the context for the primary module
+          // interface.
+          Module *FM = D->getOwningModule();
+          if (FM && !FM->isHeaderLikeModule()) {
+            // LookupModules will contain all the modules we visited during the
+            // template instantiation (if any). And if LookupModules contains
+            // FM, the declarations in FM should be visible for the
+            // instantiation.
+            const auto &LookupModules = getLookupModules();
+            // Note: We can't use 'Module::isModuleVisible()' since that is for
+            // clang modules.
+            if (LookupModules.count(FM->getTopLevelModule())) {
+              Visible = true;
+              break;
+            }
+          }
+
+          if (D->isInExportDeclContext()) {
             // C++20 [basic.lookup.argdep] p4.3 .. are exported ...
-            Module *FM = D->getOwningModule();
             // exports are only valid in module purview and outside of any
             // PMF (although a PMF should not even be present in a module
             // with an import).

diff  --git a/clang/test/Modules/named-modules-adl-2.cppm b/clang/test/Modules/named-modules-adl-2.cppm
new file mode 100644
index 0000000000000..1203e693bc4f7
--- /dev/null
+++ b/clang/test/Modules/named-modules-adl-2.cppm
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=%t/a.pcm -emit-module-interface -o %t/b.pcm
+// RUN: %clang_cc1 -std=c++20 %t/c.cppm -fmodule-file=%t/b.pcm -fsyntax-only -verify
+
+//--- a.cppm
+export module a;
+
+export template<typename T>
+void a(T x) {
+	+x;
+}
+
+//--- b.h
+struct s {
+};
+void operator+(s) {
+}
+
+//--- b.cppm
+module;
+#include "b.h"
+export module b;
+import a;
+
+export template<typename T>
+void b() {
+	a(s());
+}
+
+//--- c.cppm
+// expected-no-diagnostics
+export module c;
+import b;
+
+void c() {
+	b<int>();
+}

diff  --git a/clang/test/Modules/named-modules-adl.cppm b/clang/test/Modules/named-modules-adl.cppm
new file mode 100644
index 0000000000000..91f373071ee01
--- /dev/null
+++ b/clang/test/Modules/named-modules-adl.cppm
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -fmodule-file=%t/a.pcm -fsyntax-only -verify
+
+//--- a.h
+namespace n {
+
+struct s { };
+
+void operator+(s, int) {
+}
+
+} // namespace n
+
+//--- a.cppm
+module;
+#include "a.h"
+export module a;
+
+export template<typename T>
+void a(T x) {
+	n::s() + x;
+}
+
+//--- b.cppm
+// expected-no-diagnostics
+export module b;
+import a;
+
+void b() {
+	a(0);
+}