[clang] f09050f - [C++20] [Modules] Fix module local lookup ambiguousity

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2025-06-12T11:48:09+08:00
New Revision: f09050fdc85074869f0b34f0d9e061a74ef549ee

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

LOG: [C++20] [Modules] Fix module local lookup ambiguousity

Close https://github.com/llvm/llvm-project/issues/61360
Close https://github.com/llvm/llvm-project/issues/129525
Close https://github.com/llvm/llvm-project/issues/143734

We shouldn't identify different module local decls in different modules
as the same entity.

Added: 
    clang/test/Modules/module-local-declarations-02.cppm
    clang/test/Modules/pr61360.cppm

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/include/clang/AST/DeclBase.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/AST/DeclBase.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 8d24d393eab09..3abb49312255a 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -488,8 +488,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// if possible.
   ///
   /// Not serialized intentionally.
-  llvm::StringMap<const Module *> PrimaryModuleNameMap;
-  llvm::DenseMap<const Module *, const Module *> SameModuleLookupSet;
+  mutable llvm::StringMap<const Module *> PrimaryModuleNameMap;
+  mutable llvm::DenseMap<const Module *, const Module *> SameModuleLookupSet;
 
   static constexpr unsigned ConstantArrayTypesLog2InitSize = 8;
   static constexpr unsigned GeneralTypesLog2InitSize = 9;
@@ -1151,7 +1151,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
   ///
   /// FIXME: The signature may be confusing since `clang::Module` means to
   /// a module fragment or a module unit but not a C++20 module.
-  bool isInSameModule(const Module *M1, const Module *M2);
+  bool isInSameModule(const Module *M1, const Module *M2) const;
 
   TranslationUnitDecl *getTranslationUnitDecl() const {
     return TUDecl->getMostRecentDecl();

diff  --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 375e9e2592502..dd67ebc9873ff 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -646,6 +646,10 @@ class alignas(8) Decl {
     return getModuleOwnershipKind() == ModuleOwnershipKind::ModulePrivate;
   }
 
+  /// Whether this declaration was a local declaration to a C++20
+  /// named module.
+  bool isModuleLocal() const;
+
   /// Whether this declaration was exported in a lexical context.
   /// e.g.:
   ///

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b51f7622288df..4d44f23c0f503 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1175,7 +1175,7 @@ void ASTContext::setCurrentNamedModule(Module *M) {
   CurrentCXXNamedModule = M;
 }
 
-bool ASTContext::isInSameModule(const Module *M1, const Module *M2) {
+bool ASTContext::isInSameModule(const Module *M1, const Module *M2) const {
   if (!M1 != !M2)
     return false;
 
@@ -7429,6 +7429,12 @@ bool ASTContext::isSameEntity(const NamedDecl *X, const NamedDecl *Y) const {
                           cast<Decl>(Y->getDeclContext()->getRedeclContext())))
     return false;
 
+  // If either X or Y are local to the owning module, they are only possible to
+  // be the same entity if they are in the same module.
+  if (X->isModuleLocal() || Y->isModuleLocal())
+    if (!isInSameModule(X->getOwningModule(), Y->getOwningModule()))
+      return false;
+
   // Two typedefs refer to the same entity if they have the same underlying
   // type.
   if (const auto *TypedefX = dyn_cast<TypedefNameDecl>(X))

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index a1bb62bcb68fa..48c60aa4e449a 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1132,6 +1132,12 @@ bool Decl::isInExportDeclContext() const {
   return isa_and_nonnull<ExportDecl>(DC);
 }
 
+bool Decl::isModuleLocal() const {
+  auto *M = getOwningModule();
+  return M && M->isNamedModule() &&
+         getModuleOwnershipKind() == ModuleOwnershipKind::ReachableWhenImported;
+}
+
 bool Decl::isInAnotherModuleUnit() const {
   auto *M = getOwningModule();
 

diff  --git a/clang/test/Modules/module-local-declarations-02.cppm b/clang/test/Modules/module-local-declarations-02.cppm
new file mode 100644
index 0000000000000..0670c4295abc7
--- /dev/null
+++ b/clang/test/Modules/module-local-declarations-02.cppm
@@ -0,0 +1,31 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %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 -fprebuilt-module-path=%t -emit-llvm -o %t/B.ll
+
+//--- A.cppm
+export module A;
+
+export template<typename>
+struct holder {
+};
+
+struct foo {};
+
+export struct a {
+	holder<foo> m;
+};
+
+//--- B.cppm
+// expected-no-diagnostics
+export module B;
+
+import A;
+
+struct foo {};
+
+struct b {
+	holder<foo> m;
+};
\ No newline at end of file

diff  --git a/clang/test/Modules/pr61360.cppm b/clang/test/Modules/pr61360.cppm
new file mode 100644
index 0000000000000..a16f65d4be2fe
--- /dev/null
+++ b/clang/test/Modules/pr61360.cppm
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %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 -fprebuilt-module-path=%t -emit-llvm -o %t/B.ll
+
+//--- A.cppm
+export module A;
+export template<typename>
+struct holder {
+};
+
+struct a {
+	holder<struct foo> m;
+};
+
+//--- B.cppm
+// expected-no-diagnostics
+export module B;
+import A;
+
+struct b {
+	holder<struct foo> m;
+};