[Lldb-commits] [clang] [lldb] [AST] Add OriginalDC argument to ExternalASTSource::FindExternalVisibleDeclsByName (PR #123152)
Chuanqi Xu via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 15 19:34:21 PST 2025
https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/123152
Part for relanding https://github.com/llvm/llvm-project/pull/122887.
I split this to test where the performance regession comes from if modules are not used.
>From 15e0af0c7a7f93a2f4fce6c996ef50726770a4ea Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 16 Jan 2025 11:30:30 +0800
Subject: [PATCH] [AST] Add OriginalDC argument to
ExternalASTSource::FindExternalVisibleDeclsByName
Part of https://github.com/llvm/llvm-project/pull/122887.
I split this to test where the performance regession comes from if
modules are not used.
---
clang/include/clang/AST/DeclBase.h | 2 ++
clang/include/clang/AST/ExternalASTMerger.h | 3 ++-
clang/include/clang/AST/ExternalASTSource.h | 10 +++++++++-
.../clang/Sema/MultiplexExternalSemaSource.h | 3 ++-
clang/include/clang/Serialization/ASTReader.h | 3 ++-
clang/lib/AST/DeclBase.cpp | 15 ++++++++++-----
clang/lib/AST/ExternalASTMerger.cpp | 3 ++-
clang/lib/AST/ExternalASTSource.cpp | 3 ++-
clang/lib/Interpreter/CodeCompletion.cpp | 6 ++++--
clang/lib/Sema/MultiplexExternalSemaSource.cpp | 5 +++--
clang/lib/Serialization/ASTReader.cpp | 3 ++-
clang/unittests/AST/ExternalASTSourceTest.cpp | 3 ++-
.../Plugins/ExpressionParser/Clang/ASTUtils.h | 10 ++++++----
.../ExpressionParser/Clang/ClangASTSource.cpp | 5 +++--
.../ExpressionParser/Clang/ClangASTSource.h | 8 +++++---
.../Clang/ClangExternalASTSourceCallbacks.cpp | 3 ++-
.../Clang/ClangExternalASTSourceCallbacks.h | 3 ++-
.../ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp | 3 ++-
18 files changed, 62 insertions(+), 29 deletions(-)
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 71ab9178509b2f..840e0983311ad1 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2722,6 +2722,8 @@ class DeclContext {
bool Deserialize = false) const;
private:
+ lookup_result lookupImpl(DeclarationName Name, const DeclContext *OriginalLookupDC) 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
diff --git a/clang/include/clang/AST/ExternalASTMerger.h b/clang/include/clang/AST/ExternalASTMerger.h
index ec4cfbe2175c02..2c6f2a941311bd 100644
--- a/clang/include/clang/AST/ExternalASTMerger.h
+++ b/clang/include/clang/AST/ExternalASTMerger.h
@@ -141,7 +141,8 @@ class ExternalASTMerger : public ExternalASTSource {
/// Implementation of the ExternalASTSource API.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
- DeclarationName Name) override;
+ DeclarationName Name,
+ const DeclContext *OriginalDC) override;
/// Implementation of the ExternalASTSource API.
void
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index 4d7ff822fceb75..ac2ad72ef1e257 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -145,12 +145,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.
+ /// \param DC The context for lookup in. \c DC should be a primary context.
+ /// \param Name The name to look for.
+ /// \param OriginalDC The original context for lookup. \c OriginalDC can
+ /// provide more information than \c DC. e.g., The same namespace can appear
+ /// in multiple module units. So we need the \c OriginalDC to tell us what
+ /// the module the lookup come from.
+ ///
/// \return \c true if any declarations might have been found, \c false if
/// we definitely have no declarations with tbis name.
///
/// The default implementation of this method is a no-op returning \c false.
virtual bool
- FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name);
+ FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name,
+ const DeclContext *OriginalDC);
/// Load all the external specializations for the Decl \param D if \param
/// OnlyPartial is false. Otherwise, load all the external **partial**
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 0c92c52854c9e7..921bebe3a44af5 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -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,
+ const DeclContext *OriginalDC) override;
bool LoadExternalSpecializations(const Decl *D, bool OnlyPartial) override;
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 9f978762a6fb6b..6479a81189f905 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2119,7 +2119,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,
+ const DeclContext *OriginalDC) override;
/// Read all of the declarations lexically stored in a
/// declaration context.
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index fb701f76231bcd..3b332e68e65a1a 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1856,9 +1856,14 @@ DeclContext::lookup(DeclarationName Name) const {
if (getDeclKind() == Decl::LinkageSpec || getDeclKind() == Decl::Export)
return getParent()->lookup(Name);
- const DeclContext *PrimaryContext = getPrimaryContext();
- if (PrimaryContext != this)
- return PrimaryContext->lookup(Name);
+ return getPrimaryContext()->lookupImpl(Name, this);
+}
+
+DeclContext::lookup_result
+DeclContext::lookupImpl(DeclarationName Name, const DeclContext *OriginalLookupDC) const {
+ assert(this == getPrimaryContext() && "lookupImpl should only be called with primary DC!");
+ assert(getDeclKind() != Decl::LinkageSpec && getDeclKind() != Decl::Export &&
+ "We shouldn't lookup in transparent DC.");
// 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
@@ -1889,7 +1894,7 @@ 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, OriginalLookupDC) || !R.second) {
if (StoredDeclsMap *Map = LookupPtr) {
StoredDeclsMap::iterator I = Map->find(Name);
if (I != Map->end())
@@ -2115,7 +2120,7 @@ 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->getDeclContext());
// Insert this declaration into the map.
StoredDeclsList &DeclNameEntries = (*Map)[D->getDeclName()];
diff --git a/clang/lib/AST/ExternalASTMerger.cpp b/clang/lib/AST/ExternalASTMerger.cpp
index 7f7816e1b10eae..6639849da3f372 100644
--- a/clang/lib/AST/ExternalASTMerger.cpp
+++ b/clang/lib/AST/ExternalASTMerger.cpp
@@ -472,7 +472,8 @@ static bool importSpecializationsIfNeeded(Decl *D, ASTImporter *Importer) {
}
bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC,
- DeclarationName Name) {
+ DeclarationName Name,
+ const DeclContext *OriginalDC) {
llvm::SmallVector<NamedDecl *, 1> Decls;
llvm::SmallVector<Candidate, 4> Candidates;
diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp
index 543846c0093af8..1ca6fafd398558 100644
--- a/clang/lib/AST/ExternalASTSource.cpp
+++ b/clang/lib/AST/ExternalASTSource.cpp
@@ -92,7 +92,8 @@ ExternalASTSource::GetExternalCXXBaseSpecifiers(uint64_t Offset) {
bool
ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
- DeclarationName Name) {
+ DeclarationName Name,
+ const DeclContext *OriginalDC) {
return false;
}
diff --git a/clang/lib/Interpreter/CodeCompletion.cpp b/clang/lib/Interpreter/CodeCompletion.cpp
index bbc8830d76bc00..c0e3e604753393 100644
--- a/clang/lib/Interpreter/CodeCompletion.cpp
+++ b/clang/lib/Interpreter/CodeCompletion.cpp
@@ -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,
+ const DeclContext *OriginalDC) override;
void
completeVisibleDeclsMap(const clang::DeclContext *childDeclContext) override;
};
@@ -271,7 +272,8 @@ ExternalSource::ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM,
}
bool ExternalSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
- DeclarationName Name) {
+ DeclarationName Name,
+ const DeclContext *OriginalDC) {
IdentifierTable &ParentIdTable = ParentASTCtxt.Idents;
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 54944267b4868a..bc8d6415af135e 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -108,10 +108,11 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) {
}
bool MultiplexExternalSemaSource::
-FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) {
+FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name,
+ const DeclContext *OriginalDC) {
bool AnyDeclsFound = false;
for (size_t i = 0; i < Sources.size(); ++i)
- AnyDeclsFound |= Sources[i]->FindExternalVisibleDeclsByName(DC, Name);
+ AnyDeclsFound |= Sources[i]->FindExternalVisibleDeclsByName(DC, Name, OriginalDC);
return AnyDeclsFound;
}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 7361cace49dd7b..38c7e45ee79bf5 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8368,7 +8368,8 @@ void ASTReader::FindFileRegionDecls(FileID File,
bool
ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
- DeclarationName Name) {
+ DeclarationName Name,
+ const DeclContext *OriginalDC) {
assert(DC->hasExternalVisibleStorage() && DC == DC->getPrimaryContext() &&
"DeclContext has no visible decls in storage");
if (!Name)
diff --git a/clang/unittests/AST/ExternalASTSourceTest.cpp b/clang/unittests/AST/ExternalASTSourceTest.cpp
index 8e1bde1247f660..e70e5c9f5ba7dd 100644
--- a/clang/unittests/AST/ExternalASTSourceTest.cpp
+++ b/clang/unittests/AST/ExternalASTSourceTest.cpp
@@ -68,7 +68,8 @@ TEST(ExternalASTSourceTest, FailedLookupOccursOnce) {
TestSource(unsigned &Calls) : Calls(Calls) {}
bool FindExternalVisibleDeclsByName(const DeclContext *,
- DeclarationName Name) override {
+ DeclarationName Name,
+ const DeclContext *OriginalDC) override {
if (Name.getAsString() == "j")
++Calls;
return false;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index d5c68a436e0903..47fddb87b04c97 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -71,8 +71,9 @@ class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
}
bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
- clang::DeclarationName Name) override {
- return m_Source->FindExternalVisibleDeclsByName(DC, Name);
+ clang::DeclarationName Name,
+ const clang::DeclContext *OriginalDC) override {
+ return m_Source->FindExternalVisibleDeclsByName(DC, Name, OriginalDC);
}
bool LoadExternalSpecializations(const clang::Decl *D,
@@ -388,9 +389,10 @@ class SemaSourceWithPriorities : public clang::ExternalSemaSource {
}
bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
- clang::DeclarationName Name) override {
+ clang::DeclarationName Name,
+ const clang::DeclContext *OriginalDC) override {
for (size_t i = 0; i < Sources.size(); ++i)
- if (Sources[i]->FindExternalVisibleDeclsByName(DC, Name))
+ if (Sources[i]->FindExternalVisibleDeclsByName(DC, Name, OriginalDC))
return true;
return false;
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index e41efdd3f61c75..a10620b78a8f8e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -99,9 +99,10 @@ void ClangASTSource::StartTranslationUnit(ASTConsumer *Consumer) {
// The core lookup interface.
bool ClangASTSource::FindExternalVisibleDeclsByName(
- const DeclContext *decl_ctx, DeclarationName clang_decl_name) {
+ const DeclContext *decl_ctx, DeclarationName clang_decl_name,
+ const clang::DeclContext *original_dc) {
if (!m_ast_context) {
- SetNoExternalVisibleDeclsForName(decl_ctx, clang_decl_name);
+ SetNoExternalVisibleDeclsForName(decl_ctx, clang_decl_name, original_dc);
return false;
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
index 83c910477acc8d..87f7b951644e96 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -84,7 +84,8 @@ class ClangASTSource : public clang::ExternalASTSource,
/// \return
/// Whatever SetExternalVisibleDeclsForName returns.
bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
- clang::DeclarationName Name) override;
+ clang::DeclarationName Name,
+ const clang::DeclContext *OriginalDC) override;
/// Enumerate all Decls in a given lexical context.
///
@@ -212,8 +213,9 @@ class ClangASTSource : public clang::ExternalASTSource,
ClangASTSourceProxy(ClangASTSource &original) : m_original(original) {}
bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
- clang::DeclarationName Name) override {
- return m_original.FindExternalVisibleDeclsByName(DC, Name);
+ clang::DeclarationName Name,
+ const clang::DeclContext *OriginalDC) override {
+ return m_original.FindExternalVisibleDeclsByName(DC, Name, OriginalDC);
}
void FindExternalLexicalDecls(
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
index e746e6afe39bea..3eddf49a8b7e7a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp
@@ -50,7 +50,8 @@ void ClangExternalASTSourceCallbacks::FindExternalLexicalDecls(
}
bool ClangExternalASTSourceCallbacks::FindExternalVisibleDeclsByName(
- const clang::DeclContext *DC, clang::DeclarationName Name) {
+ const clang::DeclContext *DC, clang::DeclarationName Name,
+ const clang::DeclContext *OriginalDC) {
llvm::SmallVector<clang::NamedDecl *, 4> decls;
// Objective-C methods are not added into the LookupPtr when they originate
// from an external source. SetExternalVisibleDeclsForName() adds them.
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
index 6bd18186a567d9..ef98dc2332a5d0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h
@@ -38,7 +38,8 @@ class ClangExternalASTSourceCallbacks : public clang::ExternalASTSource {
llvm::SmallVectorImpl<clang::Decl *> &Result) override;
bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC,
- clang::DeclarationName Name) override;
+ clang::DeclarationName Name,
+ const clang::DeclContext *OriginalDC) override;
void CompleteType(clang::TagDecl *tag_decl) override;
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
index 96a259b811b5e7..1ff9cc3eabfa60 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -30,7 +30,8 @@ class lldb_private::AppleObjCExternalASTSource
: m_decl_vendor(decl_vendor) {}
bool FindExternalVisibleDeclsByName(const clang::DeclContext *decl_ctx,
- clang::DeclarationName name) override {
+ clang::DeclarationName name,
+ const clang::DeclContext *original_dc) override {
Log *log(GetLog(
LLDBLog::Expressions)); // FIXME - a more appropriate log channel?
More information about the lldb-commits
mailing list