Skip to content

Commit 378dcf6

Browse files
authored
[C++20] [Modules] Fix may-be incorrect ADL for module local entities (#123931)
Close #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.
1 parent 0ef39a8 commit 378dcf6

File tree

2 files changed

+94
-1
lines changed

2 files changed

+94
-1
lines changed

clang/lib/Sema/SemaLookup.cpp

+51-1
Original file line numberDiff line numberDiff line change
@@ -2914,7 +2914,57 @@ static void CollectEnclosingNamespace(Sema::AssociatedNamespaceSet &Namespaces,
29142914
while (!Ctx->isFileContext() || Ctx->isInlineNamespace())
29152915
Ctx = Ctx->getParent();
29162916

2917-
Namespaces.insert(Ctx->getPrimaryContext());
2917+
// Actually it is fine to always do `Namespaces.insert(Ctx);` simply. But it
2918+
// may cause more allocations in Namespaces and more unnecessary lookups. So
2919+
// we'd like to insert the representative namespace only.
2920+
DeclContext *PrimaryCtx = Ctx->getPrimaryContext();
2921+
Decl *PrimaryD = cast<Decl>(PrimaryCtx);
2922+
Decl *D = cast<Decl>(Ctx);
2923+
ASTContext &AST = D->getASTContext();
2924+
2925+
// TODO: Technically it is better to insert one namespace per module. e.g.,
2926+
//
2927+
// ```
2928+
// //--- first.cppm
2929+
// export module first;
2930+
// namespace ns { ... } // first namespace
2931+
//
2932+
// //--- m-partA.cppm
2933+
// export module m:partA;
2934+
// import first;
2935+
//
2936+
// namespace ns { ... }
2937+
// namespace ns { ... }
2938+
//
2939+
// //--- m-partB.cppm
2940+
// export module m:partB;
2941+
// import first;
2942+
// import :partA;
2943+
//
2944+
// namespace ns { ... }
2945+
// namespace ns { ... }
2946+
//
2947+
// ...
2948+
//
2949+
// //--- m-partN.cppm
2950+
// export module m:partN;
2951+
// import first;
2952+
// import :partA;
2953+
// ...
2954+
// import :part$(N-1);
2955+
//
2956+
// namespace ns { ... }
2957+
// namespace ns { ... }
2958+
//
2959+
// consume(ns::any_decl); // the lookup
2960+
// ```
2961+
//
2962+
// We should only insert once for all namespaces in module m.
2963+
if (D->isInNamedModule() &&
2964+
!AST.isInSameModule(D->getOwningModule(), PrimaryD->getOwningModule()))
2965+
Namespaces.insert(Ctx);
2966+
else
2967+
Namespaces.insert(PrimaryCtx);
29182968
}
29192969

29202970
// Add the associated classes and namespaces for argument-dependent
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: cd %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
6+
// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
7+
// RUN: -fmodule-file=a=%t/a.pcm
8+
// RUN: %clang_cc1 -std=c++20 %t/use.cc -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
9+
// RUN: -fsyntax-only -verify
10+
//
11+
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-reduced-module-interface -o %t/a.pcm
12+
// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-reduced-module-interface -o %t/b.pcm \
13+
// RUN: -fmodule-file=a=%t/a.pcm
14+
// RUN: %clang_cc1 -std=c++20 %t/use.cc -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
15+
// RUN: -fsyntax-only -verify
16+
17+
//--- a.cppm
18+
export module a;
19+
20+
namespace n {
21+
}
22+
23+
//--- b.cppm
24+
export module b;
25+
import a;
26+
27+
namespace n {
28+
struct monostate {
29+
friend bool operator==(monostate, monostate) = default;
30+
};
31+
32+
export struct wrapper {
33+
friend bool operator==(wrapper const &, wrapper const &) = default;
34+
35+
monostate m_value;
36+
};
37+
}
38+
39+
//--- use.cc
40+
// expected-no-diagnostics
41+
import b;
42+
43+
static_assert(n::wrapper() == n::wrapper());

0 commit comments

Comments
 (0)