Skip to content

Commit 01dae66

Browse files
committed
[C++20] [Modules] Support module level lookup
Close #90154 This patch is also an optimization to the lookup process to utilize the information provided by `export` keyword. Previously, in the lookup process, the `export` keyword only takes part in the check part, it doesn't get involved in the lookup process. That said, previously, in a name lookup for 'name', we would load all of declarations with the name 'name' and check if these declarations are valid or not. It works well. But it is inefficient since it may load declarations that may not be wanted. Note that this patch actually did a trick in the lookup process instead of bring module information to DeclarationName or considering module information when deciding if two declarations are the same. So it may not be a surprise to me if there are missing cases. But it is not a regression. It should be already the case. Issue reports are welcomed. In this patch, I tried to split the big lookup table into a lookup table as before and a module local lookup table, which takes a combination of the ID of the DeclContext and hash value of the primary module name as the key. And refactored `DeclContext::lookup()` method to take the module information. So that a lookup in a DeclContext won't load declarations that are local to **other** modules. And also I think it is already beneficial to split the big lookup table since it may reduce the conflicts during lookups in the hash table. BTW, this patch introduced a **regression** for a reachability rule in C++20 but it was false-negative. See 'clang/test/CXX/module/module.interface/p7.cpp' for details. This patch is not expected to introduce any other regressions for non-c++20-modules users since the module local lookup table should be empty for them.
1 parent 5870815 commit 01dae66

35 files changed

+720
-195
lines changed

clang/docs/ReleaseNotes.rst

+2
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ C++23 Feature Support
316316
C++20 Feature Support
317317
^^^^^^^^^^^^^^^^^^^^^
318318

319+
- Implemented module level lookup for C++20 modules. (#GH90154)
320+
319321

320322
Resolutions to C++ Defect Reports
321323
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/AST/DeclBase.h

+10
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,10 @@ class alignas(8) Decl {
836836
return isFromASTFile() ? getImportedOwningModule() : getLocalOwningModule();
837837
}
838838

839+
/// Get the top level owning named module that owns this declaration if any.
840+
/// \returns nullptr if the declaration is not owned by a named module.
841+
Module *getTopLevelOwningNamedModule() const;
842+
839843
/// Get the module that owns this declaration for linkage purposes.
840844
/// There only ever is such a standard C++ module.
841845
Module *getOwningModuleForLinkage() const;
@@ -2711,6 +2715,12 @@ class DeclContext {
27112715
bool Deserialize = false) const;
27122716

27132717
private:
2718+
/// Lookup all external visible declarations and the external declarations
2719+
/// within the same module specified by \param NamedModule. We can't
2720+
/// get it from \param this since the same declaration may be declared in
2721+
/// multiple modules. e.g., namespace.
2722+
lookup_result lookupImpl(DeclarationName Name, Module *NamedModule) const;
2723+
27142724
/// Whether this declaration context has had externally visible
27152725
/// storage added since the last lookup. In this case, \c LookupPtr's
27162726
/// invariant may not hold and needs to be fixed before we perform

clang/include/clang/AST/ExternalASTMerger.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ class ExternalASTMerger : public ExternalASTSource {
141141

142142
/// Implementation of the ExternalASTSource API.
143143
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
144-
DeclarationName Name) override;
144+
DeclarationName Name,
145+
Module *NamedModule) override;
145146

146147
/// Implementation of the ExternalASTSource API.
147148
void

clang/include/clang/AST/ExternalASTSource.h

+8-2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ class RecordDecl;
5151
class Selector;
5252
class Stmt;
5353
class TagDecl;
54+
class Module;
5455

5556
/// Abstract interface for external sources of AST nodes.
5657
///
@@ -145,12 +146,17 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
145146
/// Find all declarations with the given name in the given context,
146147
/// and add them to the context by calling SetExternalVisibleDeclsForName
147148
/// or SetNoExternalVisibleDeclsForName.
149+
/// \param NamedModule, when this is set the external module local
150+
/// declarations within the same module of \param NamedModule will be found
151+
/// too. The \param NamedModule may be different than the owning module of
152+
/// \param DC since the same namespace can appear in multiple module units.
148153
/// \return \c true if any declarations might have been found, \c false if
149154
/// we definitely have no declarations with tbis name.
150155
///
151156
/// The default implementation of this method is a no-op returning \c false.
152-
virtual bool
153-
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
157+
virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC,
158+
DeclarationName Name,
159+
Module *NamedModule);
154160

155161
/// Load all the external specializations for the Decl \param D if \param
156162
/// OnlyPartial is false. Otherwise, load all the external **partial**

clang/include/clang/Sema/MultiplexExternalSemaSource.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
9595
/// Find all declarations with the given name in the
9696
/// given context.
9797
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
98-
DeclarationName Name) override;
98+
DeclarationName Name,
99+
Module *NamedModule) override;
99100

100101
bool LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
101102

clang/include/clang/Serialization/ASTBitCodes.h

+6
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,8 @@ enum ASTRecordTypes {
738738
CXX_ADDED_TEMPLATE_SPECIALIZATION = 74,
739739

740740
CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION = 75,
741+
742+
UPDATE_MODULE_LOCAL_VISIBLE = 76,
741743
};
742744

743745
/// Record types used within a source manager block.
@@ -1334,6 +1336,10 @@ enum DeclCode {
13341336
/// into a DeclContext via DeclContext::lookup.
13351337
DECL_CONTEXT_VISIBLE,
13361338

1339+
/// A record containing the set of declarations that are
1340+
/// only visible from DeclContext in the same module.
1341+
DECL_CONTEXT_MODULE_LOCAL_VISIBLE,
1342+
13371343
/// A LabelDecl record.
13381344
DECL_LABEL,
13391345

clang/include/clang/Serialization/ASTReader.h

+28-4
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ class ASTIdentifierLookupTrait;
353353

354354
/// The on-disk hash table(s) used for DeclContext name lookup.
355355
struct DeclContextLookupTable;
356+
struct ModuleLocalLookupTable;
356357

357358
/// The on-disk hash table(s) used for specialization decls.
358359
struct LazySpecializationInfoLookupTable;
@@ -523,9 +524,14 @@ class ASTReader
523524
/// in the chain.
524525
DeclUpdateOffsetsMap DeclUpdateOffsets;
525526

527+
struct LookupBlockOffsets {
528+
uint64_t LexicalOffset;
529+
uint64_t VisibleOffset;
530+
uint64_t ModuleLocalOffset;
531+
};
532+
526533
using DelayedNamespaceOffsetMapTy =
527-
llvm::DenseMap<GlobalDeclID, std::pair</*LexicalOffset*/ uint64_t,
528-
/*VisibleOffset*/ uint64_t>>;
534+
llvm::DenseMap<GlobalDeclID, LookupBlockOffsets>;
529535

530536
/// Mapping from global declaration IDs to the lexical and visible block
531537
/// offset for delayed namespace in reduced BMI.
@@ -631,6 +637,9 @@ class ASTReader
631637
/// Map from a DeclContext to its lookup tables.
632638
llvm::DenseMap<const DeclContext *,
633639
serialization::reader::DeclContextLookupTable> Lookups;
640+
llvm::DenseMap<const DeclContext *,
641+
serialization::reader::ModuleLocalLookupTable>
642+
ModuleLocalLookups;
634643

635644
using SpecLookupTableTy =
636645
llvm::DenseMap<const Decl *,
@@ -659,6 +668,8 @@ class ASTReader
659668
/// Updates to the visible declarations of declaration contexts that
660669
/// haven't been loaded yet.
661670
llvm::DenseMap<GlobalDeclID, DeclContextVisibleUpdates> PendingVisibleUpdates;
671+
llvm::DenseMap<GlobalDeclID, DeclContextVisibleUpdates>
672+
PendingModuleLocalVisibleUpdates;
662673

663674
using SpecializationsUpdate = SmallVector<UpdateData, 1>;
664675
using SpecializationsUpdateMap =
@@ -696,7 +707,8 @@ class ASTReader
696707
/// Read the record that describes the visible contents of a DC.
697708
bool ReadVisibleDeclContextStorage(ModuleFile &M,
698709
llvm::BitstreamCursor &Cursor,
699-
uint64_t Offset, GlobalDeclID ID);
710+
uint64_t Offset, GlobalDeclID ID,
711+
bool IsModuleLocal);
700712

701713
bool ReadSpecializations(ModuleFile &M, llvm::BitstreamCursor &Cursor,
702714
uint64_t Offset, Decl *D, bool IsPartial);
@@ -1132,6 +1144,10 @@ class ASTReader
11321144
/// Number of visible decl contexts read/total.
11331145
unsigned NumVisibleDeclContextsRead = 0, TotalVisibleDeclContexts = 0;
11341146

1147+
/// Number of module local visible decl contexts read/total.
1148+
unsigned NumModuleLocalVisibleDeclContexts = 0,
1149+
TotalModuleLocalVisibleDeclContexts = 0;
1150+
11351151
/// Total size of modules, in bits, currently loaded
11361152
uint64_t TotalModulesSizeInBits = 0;
11371153

@@ -1444,6 +1460,9 @@ class ASTReader
14441460
const serialization::reader::DeclContextLookupTable *
14451461
getLoadedLookupTables(DeclContext *Primary) const;
14461462

1463+
const serialization::reader::ModuleLocalLookupTable *
1464+
getModuleLocalLookupTables(DeclContext *Primary) const;
1465+
14471466
/// Get the loaded specializations lookup tables for \p D,
14481467
/// if any.
14491468
serialization::reader::LazySpecializationInfoLookupTable *
@@ -2119,7 +2138,8 @@ class ASTReader
21192138
/// The current implementation of this method just loads the entire
21202139
/// lookup table as unmaterialized references.
21212140
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
2122-
DeclarationName Name) override;
2141+
DeclarationName Name,
2142+
Module *NamedModule) override;
21232143

21242144
/// Read all of the declarations lexically stored in a
21252145
/// declaration context.
@@ -2607,6 +2627,10 @@ inline bool shouldSkipCheckingODR(const Decl *D) {
26072627
(D->isFromGlobalModule() || D->isFromHeaderUnit());
26082628
}
26092629

2630+
/// Calculate a hash value for the primary module name of the given module.
2631+
/// \returns std::nullopt if M is not a C++ standard module.
2632+
std::optional<unsigned> getPrimaryModuleHash(const Module *M);
2633+
26102634
} // namespace clang
26112635

26122636
#endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H

clang/include/clang/Serialization/ASTWriter.h

+13-3
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,10 @@ class ASTWriter : public ASTDeserializationListener,
492492
/// file.
493493
unsigned NumVisibleDeclContexts = 0;
494494

495+
/// The number of module local visible declcontexts written to the AST
496+
/// file.
497+
unsigned NumModuleLocalDeclContexts = 0;
498+
495499
/// A mapping from each known submodule to its ID number, which will
496500
/// be a positive integer.
497501
llvm::DenseMap<const Module *, unsigned> SubmoduleIDs;
@@ -587,11 +591,15 @@ class ASTWriter : public ASTDeserializationListener,
587591
uint64_t WriteSpecializationInfoLookupTable(
588592
const NamedDecl *D, llvm::SmallVectorImpl<const Decl *> &Specializations,
589593
bool IsPartial);
590-
void GenerateNameLookupTable(ASTContext &Context, const DeclContext *DC,
591-
llvm::SmallVectorImpl<char> &LookupTable);
594+
void
595+
GenerateNameLookupTable(ASTContext &Context, const DeclContext *DC,
596+
llvm::SmallVectorImpl<char> &LookupTable,
597+
llvm::SmallVectorImpl<char> &ModuleLocalLookupTable);
592598
uint64_t WriteDeclContextLexicalBlock(ASTContext &Context,
593599
const DeclContext *DC);
594-
uint64_t WriteDeclContextVisibleBlock(ASTContext &Context, DeclContext *DC);
600+
void WriteDeclContextVisibleBlock(ASTContext &Context, DeclContext *DC,
601+
uint64_t &VisibleBlockOffset,
602+
uint64_t &ModuleLocalBlockOffset);
595603
void WriteTypeDeclOffsets();
596604
void WriteFileDeclIDsMap();
597605
void WriteComments(ASTContext &Context);
@@ -624,7 +632,9 @@ class ASTWriter : public ASTDeserializationListener,
624632
unsigned DeclParmVarAbbrev = 0;
625633
unsigned DeclContextLexicalAbbrev = 0;
626634
unsigned DeclContextVisibleLookupAbbrev = 0;
635+
unsigned DeclModuleLocalVisibleLookupAbbrev = 0;
627636
unsigned UpdateVisibleAbbrev = 0;
637+
unsigned ModuleLocalUpdateVisibleAbbrev = 0;
628638
unsigned DeclRecordAbbrev = 0;
629639
unsigned DeclTypedefAbbrev = 0;
630640
unsigned DeclVarAbbrev = 0;

clang/lib/AST/DeclBase.cpp

+19-4
Original file line numberDiff line numberDiff line change
@@ -1850,15 +1850,28 @@ void DeclContext::buildLookupImpl(DeclContext *DCtx, bool Internal) {
18501850
}
18511851
}
18521852

1853+
Module *Decl::getTopLevelOwningNamedModule() const {
1854+
if (getOwningModule() &&
1855+
getOwningModule()->getTopLevelModule()->isNamedModule())
1856+
return getOwningModule()->getTopLevelModule();
1857+
1858+
return nullptr;
1859+
}
1860+
18531861
DeclContext::lookup_result
18541862
DeclContext::lookup(DeclarationName Name) const {
1863+
return lookupImpl(Name, cast<Decl>(this)->getTopLevelOwningNamedModule());
1864+
}
1865+
1866+
DeclContext::lookup_result DeclContext::lookupImpl(DeclarationName Name,
1867+
Module *NamedModule) const {
18551868
// For transparent DeclContext, we should lookup in their enclosing context.
18561869
if (getDeclKind() == Decl::LinkageSpec || getDeclKind() == Decl::Export)
1857-
return getParent()->lookup(Name);
1870+
return getParent()->lookupImpl(Name, NamedModule);
18581871

18591872
const DeclContext *PrimaryContext = getPrimaryContext();
18601873
if (PrimaryContext != this)
1861-
return PrimaryContext->lookup(Name);
1874+
return PrimaryContext->lookupImpl(Name, NamedModule);
18621875

18631876
// If we have an external source, ensure that any later redeclarations of this
18641877
// context have been loaded, since they may add names to the result of this
@@ -1889,7 +1902,8 @@ DeclContext::lookup(DeclarationName Name) const {
18891902
if (!R.second && !R.first->second.hasExternalDecls())
18901903
return R.first->second.getLookupResult();
18911904

1892-
if (Source->FindExternalVisibleDeclsByName(this, Name) || !R.second) {
1905+
if (Source->FindExternalVisibleDeclsByName(this, Name, NamedModule) ||
1906+
!R.second) {
18931907
if (StoredDeclsMap *Map = LookupPtr) {
18941908
StoredDeclsMap::iterator I = Map->find(Name);
18951909
if (I != Map->end())
@@ -2115,7 +2129,8 @@ void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D, bool Internal) {
21152129
if (ExternalASTSource *Source = getParentASTContext().getExternalSource())
21162130
if (hasExternalVisibleStorage() &&
21172131
Map->find(D->getDeclName()) == Map->end())
2118-
Source->FindExternalVisibleDeclsByName(this, D->getDeclName());
2132+
Source->FindExternalVisibleDeclsByName(
2133+
this, D->getDeclName(), D->getTopLevelOwningNamedModule());
21192134

21202135
// Insert this declaration into the map.
21212136
StoredDeclsList &DeclNameEntries = (*Map)[D->getDeclName()];

clang/lib/AST/ExternalASTMerger.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,8 @@ static bool importSpecializationsIfNeeded(Decl *D, ASTImporter *Importer) {
472472
}
473473

474474
bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC,
475-
DeclarationName Name) {
475+
DeclarationName Name,
476+
Module *NamedModule) {
476477
llvm::SmallVector<NamedDecl *, 1> Decls;
477478
llvm::SmallVector<Candidate, 4> Candidates;
478479

clang/lib/AST/ExternalASTSource.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ ExternalASTSource::GetExternalCXXBaseSpecifiers(uint64_t Offset) {
9090
return nullptr;
9191
}
9292

93-
bool
94-
ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
95-
DeclarationName Name) {
93+
bool ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
94+
DeclarationName Name,
95+
Module *NamedModule) {
9696
return false;
9797
}
9898

clang/lib/Interpreter/CodeCompletion.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ class ExternalSource : public clang::ExternalASTSource {
228228
ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM,
229229
ASTContext &ParentASTCtxt, FileManager &ParentFM);
230230
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
231-
DeclarationName Name) override;
231+
DeclarationName Name,
232+
Module *NamedModule) override;
232233
void
233234
completeVisibleDeclsMap(const clang::DeclContext *childDeclContext) override;
234235
};
@@ -271,7 +272,8 @@ ExternalSource::ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM,
271272
}
272273

273274
bool ExternalSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
274-
DeclarationName Name) {
275+
DeclarationName Name,
276+
Module *NamedModule) {
275277

276278
IdentifierTable &ParentIdTable = ParentASTCtxt.Idents;
277279

clang/lib/Sema/MultiplexExternalSemaSource.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,12 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) {
107107
return EK_ReplyHazy;
108108
}
109109

110-
bool MultiplexExternalSemaSource::
111-
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) {
110+
bool MultiplexExternalSemaSource::FindExternalVisibleDeclsByName(
111+
const DeclContext *DC, DeclarationName Name, Module *NamedModule) {
112112
bool AnyDeclsFound = false;
113113
for (size_t i = 0; i < Sources.size(); ++i)
114-
AnyDeclsFound |= Sources[i]->FindExternalVisibleDeclsByName(DC, Name);
114+
AnyDeclsFound |=
115+
Sources[i]->FindExternalVisibleDeclsByName(DC, Name, NamedModule);
115116
return AnyDeclsFound;
116117
}
117118

0 commit comments

Comments
 (0)