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