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

[via cfe-commits]

Author: Chuanqi Xu
Date: 2025-01-24T10:51:10+08:00
New Revision: 378dcf61014b787b3542b917f6296c9fb5ec490c

URL: https://github.com/llvm/llvm-project/commit/378dcf61014b787b3542b917f6296c9fb5ec490c
DIFF: https://github.com/llvm/llvm-project/commit/378dcf61014b787b3542b917f6296c9fb5ec490c.diff

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

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

See the comments for details. We can't get primary context arbitrarily
since the redecl may have different context and information.

There is a TODO for modules specific case, I'd like to make it after
this PR.

Added: 
    clang/test/Modules/module-local-hidden-friend-2.cppm

Modified: 
    clang/lib/Sema/SemaLookup.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 2ed8d3608d49ec..5f8ffa71607bb0 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());