https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/123152
>From ea3eb4454319ce703bf689dac000f0ed382c2ee5 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@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 | 3 +++ clang/include/clang/AST/ExternalASTMerger.h | 3 ++- clang/include/clang/AST/ExternalASTSource.h | 12 ++++++++++-- .../clang/Sema/MultiplexExternalSemaSource.h | 3 ++- clang/include/clang/Serialization/ASTReader.h | 3 ++- clang/lib/AST/DeclBase.cpp | 19 ++++++++++++++----- clang/lib/AST/ExternalASTMerger.cpp | 5 +++-- clang/lib/AST/ExternalASTSource.cpp | 6 +++--- clang/lib/Interpreter/CodeCompletion.cpp | 8 +++++--- .../lib/Sema/MultiplexExternalSemaSource.cpp | 8 +++++--- clang/lib/Serialization/ASTReader.cpp | 6 +++--- clang/unittests/AST/ExternalASTSourceTest.cpp | 5 +++-- .../Plugins/ExpressionParser/Clang/ASTUtils.h | 14 ++++++++------ .../ExpressionParser/Clang/ClangASTSource.cpp | 3 ++- .../ExpressionParser/Clang/ClangASTSource.h | 13 ++++++++----- .../Clang/ClangExternalASTSourceCallbacks.cpp | 3 ++- .../Clang/ClangExternalASTSourceCallbacks.h | 6 ++++-- .../AppleObjCRuntime/AppleObjCDeclVendor.cpp | 5 +++-- 18 files changed, 82 insertions(+), 43 deletions(-) diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 71ab9178509b2f..a6b07dc07e25a7 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -2722,6 +2722,9 @@ 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..42aed56d42e076 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); + virtual bool 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..7c2dcf95e37922 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1856,9 +1856,16 @@ 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 +1896,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, OriginalLookupDC) || + !R.second) { if (StoredDeclsMap *Map = LookupPtr) { StoredDeclsMap::iterator I = Map->find(Name); if (I != Map->end()) @@ -2115,7 +2123,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->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..257e8338dedef3 100644 --- a/clang/lib/AST/ExternalASTMerger.cpp +++ b/clang/lib/AST/ExternalASTMerger.cpp @@ -471,8 +471,9 @@ static bool importSpecializationsIfNeeded(Decl *D, ASTImporter *Importer) { return false; } -bool ExternalASTMerger::FindExternalVisibleDeclsByName(const DeclContext *DC, - DeclarationName Name) { +bool ExternalASTMerger::FindExternalVisibleDeclsByName( + const DeclContext *DC, 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..e2451f294741d3 100644 --- a/clang/lib/AST/ExternalASTSource.cpp +++ b/clang/lib/AST/ExternalASTSource.cpp @@ -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, + const DeclContext *OriginalDC) { return false; } diff --git a/clang/lib/Interpreter/CodeCompletion.cpp b/clang/lib/Interpreter/CodeCompletion.cpp index bbc8830d76bc00..aa906635381286 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; }; @@ -270,8 +271,9 @@ ExternalSource::ExternalSource(ASTContext &ChildASTCtxt, FileManager &ChildFM, Importer.reset(importer); } -bool ExternalSource::FindExternalVisibleDeclsByName(const DeclContext *DC, - DeclarationName Name) { +bool ExternalSource::FindExternalVisibleDeclsByName( + const DeclContext *DC, 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..6d945300c386c0 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -107,11 +107,13 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) { return EK_ReplyHazy; } -bool MultiplexExternalSemaSource:: -FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) { +bool MultiplexExternalSemaSource::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..8794a0b0287873 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -8366,9 +8366,9 @@ void ASTReader::FindFileRegionDecls(FileID File, *DInfo.Mod, LocalDeclID::get(*this, *DInfo.Mod, *DIt)))); } -bool -ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, - DeclarationName Name) { +bool ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, + 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..512f21e8efff4f 100644 --- a/clang/unittests/AST/ExternalASTSourceTest.cpp +++ b/clang/unittests/AST/ExternalASTSourceTest.cpp @@ -67,8 +67,9 @@ TEST(ExternalASTSourceTest, FailedLookupOccursOnce) { struct TestSource : ExternalASTSource { TestSource(unsigned &Calls) : Calls(Calls) {} - bool FindExternalVisibleDeclsByName(const DeclContext *, - DeclarationName Name) override { + bool + FindExternalVisibleDeclsByName(const DeclContext *, 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..a1f02dc3d1b09f 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h @@ -70,9 +70,10 @@ class ExternalASTSourceWrapper : public clang::ExternalSemaSource { m_Source->updateOutOfDateIdentifier(II); } - bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, - clang::DeclarationName Name) override { - return m_Source->FindExternalVisibleDeclsByName(DC, Name); + bool FindExternalVisibleDeclsByName( + const clang::DeclContext *DC, clang::DeclarationName Name, + const clang::DeclContext *OriginalDC) override { + return m_Source->FindExternalVisibleDeclsByName(DC, Name, OriginalDC); } bool LoadExternalSpecializations(const clang::Decl *D, @@ -387,10 +388,11 @@ class SemaSourceWithPriorities : public clang::ExternalSemaSource { return EK_ReplyHazy; } - bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, - clang::DeclarationName Name) override { + bool FindExternalVisibleDeclsByName( + const clang::DeclContext *DC, 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..34129807277d5d 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -99,7 +99,8 @@ 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); return false; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h index 83c910477acc8d..dd89bae96f6293 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h @@ -83,8 +83,10 @@ class ClangASTSource : public clang::ExternalASTSource, /// /// \return /// Whatever SetExternalVisibleDeclsForName returns. - bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, - clang::DeclarationName Name) override; + bool + FindExternalVisibleDeclsByName(const clang::DeclContext *DC, + clang::DeclarationName Name, + const clang::DeclContext *OriginalDC) override; /// Enumerate all Decls in a given lexical context. /// @@ -211,9 +213,10 @@ class ClangASTSource : public clang::ExternalASTSource, public: ClangASTSourceProxy(ClangASTSource &original) : m_original(original) {} - bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, - clang::DeclarationName Name) override { - return m_original.FindExternalVisibleDeclsByName(DC, Name); + bool FindExternalVisibleDeclsByName( + const clang::DeclContext *DC, 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..d0eabb509455c1 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h @@ -37,8 +37,10 @@ class ClangExternalASTSourceCallbacks : public clang::ExternalASTSource { llvm::function_ref<bool(clang::Decl::Kind)> IsKindWeWant, llvm::SmallVectorImpl<clang::Decl *> &Result) override; - bool FindExternalVisibleDeclsByName(const clang::DeclContext *DC, - clang::DeclarationName Name) override; + bool + FindExternalVisibleDeclsByName(const clang::DeclContext *DC, + 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..e4b20b30a069f4 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp @@ -29,8 +29,9 @@ class lldb_private::AppleObjCExternalASTSource AppleObjCExternalASTSource(AppleObjCDeclVendor &decl_vendor) : m_decl_vendor(decl_vendor) {} - bool FindExternalVisibleDeclsByName(const clang::DeclContext *decl_ctx, - clang::DeclarationName name) override { + bool FindExternalVisibleDeclsByName( + const clang::DeclContext *decl_ctx, clang::DeclarationName name, + const clang::DeclContext *original_dc) override { Log *log(GetLog( LLDBLog::Expressions)); // FIXME - a more appropriate log channel? _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits