[clang] [C++20] [Modules] Fix may-be incorrect ADL for module local entities (PR #123931)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/123931

>From d83d7bd83cd6bd635420ec4e824afa299f20c154 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Wed, 22 Jan 2025 11:41:55 +0800
Subject: [PATCH] [C++20] [Modules] Fix may-be incorrect ADL for module local
 entities

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

See the comments for details
---
 clang/lib/Sema/SemaLookup.cpp                 | 52 ++++++++++++++++++-
 .../Modules/module-local-hidden-friend-2.cppm | 43 +++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/Modules/module-local-hidden-friend-2.cppm

diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index e18e3c197383e2..2751cfd6b18e96 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -2914,7 +2914,57 @@ static void CollectEnclosingNamespace(Sema::AssociatedNamespaceSet &Namespaces,
   while (!Ctx->isFileContext() || Ctx->isInlineNamespace())
     Ctx = Ctx->getParent();
 
-  Namespaces.insert(Ctx->getPrimaryContext());
+  // Actually it is fine to always do `Namespaces.insert(Ctx);` simply. But it
+  // may cause more allocations in Namespaces and more unnecessary lookups. So
+  // we'd like to insert the representative namespace only.
+  DeclContext *PrimaryCtx = Ctx->getPrimaryContext();
+  Decl *PrimaryD = cast<Decl>(PrimaryCtx);
+  Decl *D = cast<Decl>(Ctx);
+  ASTContext &AST = D->getASTContext();
+
+  // TODO: Technically it is better to insert one namespace per module. e.g.,
+  //
+  // ```
+  // //--- first.cppm
+  // export module first;
+  // namespace ns { ... } // first namespace
+  //
+  // //--- m-partA.cppm
+  // export module m:partA;
+  // import first;
+  //
+  // namespace ns { ... }
+  // namespace ns { ... }
+  //
+  // //--- m-partB.cppm
+  // export module m:partB;
+  // import first;
+  // import :partA;
+  //
+  // namespace ns { ... }
+  // namespace ns { ... }
+  //
+  // ...
+  //
+  // //--- m-partN.cppm
+  // export module m:partN;
+  // import first;
+  // import :partA;
+  // ...
+  // import :part$(N-1);
+  //
+  // namespace ns { ... }
+  // namespace ns { ... }
+  //
+  // consume(ns::any_decl); // the lookup
+  // ```
+  //
+  // We should only insert once for all namespaces in module m.
+  if (D->isInNamedModule() &&
+      !AST.isInSameModule(D->getOwningModule(), PrimaryD->getOwningModule()))
+    Namespaces.insert(Ctx);
+  else
+    Namespaces.insert(PrimaryCtx);
 }
 
 // Add the associated classes and namespaces for argument-dependent
diff --git a/clang/test/Modules/module-local-hidden-friend-2.cppm b/clang/test/Modules/module-local-hidden-friend-2.cppm
new file mode 100644
index 00000000000000..d415e495abb213
--- /dev/null
+++ b/clang/test/Modules/module-local-hidden-friend-2.cppm
@@ -0,0 +1,43 @@
+// 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 -emit-module-interface -o %t/b.pcm \
+// RUN:     -fmodule-file=a=%t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use.cc -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
+// RUN:     -fsyntax-only -verify
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-reduced-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-reduced-module-interface -o %t/b.pcm \
+// RUN:     -fmodule-file=a=%t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use.cc -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
+// RUN:     -fsyntax-only -verify
+
+//--- a.cppm
+export module a;
+
+namespace n {
+}
+
+//--- b.cppm
+export module b;
+import a;
+
+namespace n {
+struct monostate {
+	friend bool operator==(monostate, monostate) = default;
+};
+
+export struct wrapper {
+	friend bool operator==(wrapper const &, wrapper const &) = default;
+
+	monostate m_value;
+};
+}
+
+//--- use.cc
+// expected-no-diagnostics
+import b;
+
+static_assert(n::wrapper() == n::wrapper());