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