From 3ce6686a698f162eeb54ca93159291a6e1e3bf39 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd@linux.alibaba.com>
Date: Tue, 7 Feb 2023 13:58:11 +0800
Subject: [PATCH] [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.
---
 clang/lib/Sema/SemaLookup.cpp               | 42 +++++++++++++++++++--
 clang/test/Modules/named-modules-adl-2.cppm | 41 ++++++++++++++++++++
 clang/test/Modules/named-modules-adl.cppm   | 35 +++++++++++++++++
 3 files changed, 115 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Modules/named-modules-adl-2.cppm
 create mode 100644 clang/test/Modules/named-modules-adl.cppm

diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index b2e943699c5f..b910974d2ce6 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 000000000000..1203e693bc4f
--- /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 000000000000..91f373071ee0
--- /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);
+}