Skip to content

Commit

Permalink
[C++20] [Modules] Support module level lookup (#122887)
Browse files Browse the repository at this point in the history
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.

---

On the API side, this patch unfortunately add a maybe-confusing argument
`Module *NamedModule` to
`ExternalASTSource::FindExternalVisibleDeclsByName()`. People may think
we can get the information from the first argument `const DeclContext
*DC`. But sadly there are declarations (e.g., namespace) can appear in
multiple different modules as a single declaration. So we have to add
additional information to indicate this.
  • Loading branch information
ChuanqiXu9 authored Jan 15, 2025
1 parent edc0235 commit 7201cae
Show file tree
Hide file tree
Showing 35 changed files with 736 additions and 197 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ C++23 Feature Support
C++20 Feature Support
^^^^^^^^^^^^^^^^^^^^^

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


Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,10 @@ class alignas(8) Decl {
return isFromASTFile() ? getImportedOwningModule() : getLocalOwningModule();
}

/// Get the top level owning named module that owns this declaration if any.
/// \returns nullptr if the declaration is not owned by a named module.
Module *getTopLevelOwningNamedModule() const;

/// Get the module that owns this declaration for linkage purposes.
/// There only ever is such a standard C++ module.
Module *getOwningModuleForLinkage() const;
Expand Down Expand Up @@ -2722,6 +2726,12 @@ class DeclContext {
bool Deserialize = false) const;

private:
/// Lookup all external visible declarations and the external declarations
/// within the same module specified by \c NamedModule. We can't
/// get it from \c this since the same declaration may be declared in
/// multiple modules. e.g., namespace.
lookup_result lookupImpl(DeclarationName Name, Module *NamedModule) const;

/// Whether this declaration context has had externally visible
/// storage added since the last lookup. In this case, \c LookupPtr's
/// invariant may not hold and needs to be fixed before we perform
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/AST/ExternalASTMerger.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ class ExternalASTMerger : public ExternalASTSource {

/// Implementation of the ExternalASTSource API.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) override;
DeclarationName Name,
Module *NamedModule) override;

/// Implementation of the ExternalASTSource API.
void
Expand Down
17 changes: 13 additions & 4 deletions clang/include/clang/AST/ExternalASTSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class RecordDecl;
class Selector;
class Stmt;
class TagDecl;
class Module;

/// Abstract interface for external sources of AST nodes.
///
Expand Down Expand Up @@ -145,12 +146,20 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
/// Find all declarations with the given name in the given context,
/// and add them to the context by calling SetExternalVisibleDeclsForName
/// or SetNoExternalVisibleDeclsForName.
/// \return \c true if any declarations might have been found, \c false if
/// we definitely have no declarations with tbis name.
/// \param DC the context for lookup.
/// \param Name the name of the declarations to find.
/// \param NamedModule find declarations visible to the given module
/// \c NamedModule . This may be different from owning module of \c DC since
/// there are declarations (e.g., namespace declaration) can appear in
/// multiple modules.
///
/// \return \c true if any declarations might have been found, and \c false
/// if we definitely have no declarations with this name.
///
/// The default implementation of this method is a no-op returning \c false.
virtual bool
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name,
Module *NamedModule);

/// Load all the external specializations for the Decl \param D if \param
/// OnlyPartial is false. Otherwise, load all the external **partial**
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/MultiplexExternalSemaSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
/// Find all declarations with the given name in the
/// given context.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) override;
DeclarationName Name,
Module *NamedModule) override;

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

Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,8 @@ enum ASTRecordTypes {
CXX_ADDED_TEMPLATE_SPECIALIZATION = 74,

CXX_ADDED_TEMPLATE_PARTIAL_SPECIALIZATION = 75,

UPDATE_MODULE_LOCAL_VISIBLE = 76,
};

/// Record types used within a source manager block.
Expand Down Expand Up @@ -1334,6 +1336,10 @@ enum DeclCode {
/// into a DeclContext via DeclContext::lookup.
DECL_CONTEXT_VISIBLE,

/// A record containing the set of declarations that are
/// only visible from DeclContext in the same module.
DECL_CONTEXT_MODULE_LOCAL_VISIBLE,

/// A LabelDecl record.
DECL_LABEL,

Expand Down
32 changes: 28 additions & 4 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ class ASTIdentifierLookupTrait;

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

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

struct LookupBlockOffsets {
uint64_t LexicalOffset;
uint64_t VisibleOffset;
uint64_t ModuleLocalOffset;
};

using DelayedNamespaceOffsetMapTy =
llvm::DenseMap<GlobalDeclID, std::pair</*LexicalOffset*/ uint64_t,
/*VisibleOffset*/ uint64_t>>;
llvm::DenseMap<GlobalDeclID, LookupBlockOffsets>;

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

using SpecLookupTableTy =
llvm::DenseMap<const Decl *,
Expand Down Expand Up @@ -659,6 +668,8 @@ class ASTReader
/// Updates to the visible declarations of declaration contexts that
/// haven't been loaded yet.
llvm::DenseMap<GlobalDeclID, DeclContextVisibleUpdates> PendingVisibleUpdates;
llvm::DenseMap<GlobalDeclID, DeclContextVisibleUpdates>
PendingModuleLocalVisibleUpdates;

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

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

/// Number of module local visible decl contexts read/total.
unsigned NumModuleLocalVisibleDeclContexts = 0,
TotalModuleLocalVisibleDeclContexts = 0;

/// Total size of modules, in bits, currently loaded
uint64_t TotalModulesSizeInBits = 0;

Expand Down Expand Up @@ -1444,6 +1460,9 @@ class ASTReader
const serialization::reader::DeclContextLookupTable *
getLoadedLookupTables(DeclContext *Primary) const;

const serialization::reader::ModuleLocalLookupTable *
getModuleLocalLookupTables(DeclContext *Primary) const;

/// Get the loaded specializations lookup tables for \p D,
/// if any.
serialization::reader::LazySpecializationInfoLookupTable *
Expand Down Expand Up @@ -2119,7 +2138,8 @@ class ASTReader
/// The current implementation of this method just loads the entire
/// lookup table as unmaterialized references.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) override;
DeclarationName Name,
Module *NamedModule) override;

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

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

} // namespace clang

#endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H
16 changes: 13 additions & 3 deletions clang/include/clang/Serialization/ASTWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,10 @@ class ASTWriter : public ASTDeserializationListener,
/// file.
unsigned NumVisibleDeclContexts = 0;

/// The number of module local visible declcontexts written to the AST
/// file.
unsigned NumModuleLocalDeclContexts = 0;

/// A mapping from each known submodule to its ID number, which will
/// be a positive integer.
llvm::DenseMap<const Module *, unsigned> SubmoduleIDs;
Expand Down Expand Up @@ -587,11 +591,15 @@ class ASTWriter : public ASTDeserializationListener,
uint64_t WriteSpecializationInfoLookupTable(
const NamedDecl *D, llvm::SmallVectorImpl<const Decl *> &Specializations,
bool IsPartial);
void GenerateNameLookupTable(ASTContext &Context, const DeclContext *DC,
llvm::SmallVectorImpl<char> &LookupTable);
void
GenerateNameLookupTable(ASTContext &Context, const DeclContext *DC,
llvm::SmallVectorImpl<char> &LookupTable,
llvm::SmallVectorImpl<char> &ModuleLocalLookupTable);
uint64_t WriteDeclContextLexicalBlock(ASTContext &Context,
const DeclContext *DC);
uint64_t WriteDeclContextVisibleBlock(ASTContext &Context, DeclContext *DC);
void WriteDeclContextVisibleBlock(ASTContext &Context, DeclContext *DC,
uint64_t &VisibleBlockOffset,
uint64_t &ModuleLocalBlockOffset);
void WriteTypeDeclOffsets();
void WriteFileDeclIDsMap();
void WriteComments(ASTContext &Context);
Expand Down Expand Up @@ -624,7 +632,9 @@ class ASTWriter : public ASTDeserializationListener,
unsigned DeclParmVarAbbrev = 0;
unsigned DeclContextLexicalAbbrev = 0;
unsigned DeclContextVisibleLookupAbbrev = 0;
unsigned DeclModuleLocalVisibleLookupAbbrev = 0;
unsigned UpdateVisibleAbbrev = 0;
unsigned ModuleLocalUpdateVisibleAbbrev = 0;
unsigned DeclRecordAbbrev = 0;
unsigned DeclTypedefAbbrev = 0;
unsigned DeclVarAbbrev = 0;
Expand Down
23 changes: 19 additions & 4 deletions clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1850,15 +1850,28 @@ void DeclContext::buildLookupImpl(DeclContext *DCtx, bool Internal) {
}
}

Module *Decl::getTopLevelOwningNamedModule() const {
if (getOwningModule() &&
getOwningModule()->getTopLevelModule()->isNamedModule())
return getOwningModule()->getTopLevelModule();

return nullptr;
}

DeclContext::lookup_result
DeclContext::lookup(DeclarationName Name) const {
return lookupImpl(Name, cast<Decl>(this)->getTopLevelOwningNamedModule());
}

DeclContext::lookup_result DeclContext::lookupImpl(DeclarationName Name,
Module *NamedModule) const {
// For transparent DeclContext, we should lookup in their enclosing context.
if (getDeclKind() == Decl::LinkageSpec || getDeclKind() == Decl::Export)
return getParent()->lookup(Name);
return getParent()->lookupImpl(Name, NamedModule);

const DeclContext *PrimaryContext = getPrimaryContext();
if (PrimaryContext != this)
return PrimaryContext->lookup(Name);
return PrimaryContext->lookupImpl(Name, NamedModule);

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

if (Source->FindExternalVisibleDeclsByName(this, Name) || !R.second) {
if (Source->FindExternalVisibleDeclsByName(this, Name, NamedModule) ||
!R.second) {
if (StoredDeclsMap *Map = LookupPtr) {
StoredDeclsMap::iterator I = Map->find(Name);
if (I != Map->end())
Expand Down Expand Up @@ -2115,7 +2129,8 @@ void DeclContext::makeDeclVisibleInContextImpl(NamedDecl *D, bool Internal) {
if (ExternalASTSource *Source = getParentASTContext().getExternalSource())
if (hasExternalVisibleStorage() &&
Map->find(D->getDeclName()) == Map->end())
Source->FindExternalVisibleDeclsByName(this, D->getDeclName());
Source->FindExternalVisibleDeclsByName(
this, D->getDeclName(), D->getTopLevelOwningNamedModule());

// Insert this declaration into the map.
StoredDeclsList &DeclNameEntries = (*Map)[D->getDeclName()];
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/ExternalASTMerger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,8 @@ static bool importSpecializationsIfNeeded(Decl *D, ASTImporter *Importer) {
}

bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) {
DeclarationName Name,
Module *NamedModule) {
llvm::SmallVector<NamedDecl *, 1> Decls;
llvm::SmallVector<Candidate, 4> Candidates;

Expand Down
6 changes: 3 additions & 3 deletions clang/lib/AST/ExternalASTSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ ExternalASTSource::GetExternalCXXBaseSpecifiers(uint64_t Offset) {
return nullptr;
}

bool
ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) {
bool ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name,
Module *NamedModule) {
return false;
}

Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Interpreter/CodeCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ class ExternalSource : public clang::ExternalASTSource {
ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM,
ASTContext &ParentASTCtxt, FileManager &ParentFM);
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) override;
DeclarationName Name,
Module *NamedModule) override;
void
completeVisibleDeclsMap(const clang::DeclContext *childDeclContext) override;
};
Expand Down Expand Up @@ -271,7 +272,8 @@ ExternalSource::ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM,
}

bool ExternalSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) {
DeclarationName Name,
Module *NamedModule) {

IdentifierTable &ParentIdTable = ParentASTCtxt.Idents;

Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Sema/MultiplexExternalSemaSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,12 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) {
return EK_ReplyHazy;
}

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

Expand Down
Loading

0 comments on commit 7201cae

Please sign in to comment.