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; +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits