llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: Chuanqi Xu (ChuanqiXu9) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/123152.diff 18 Files Affected: - (modified) clang/include/clang/AST/DeclBase.h (+2) - (modified) clang/include/clang/AST/ExternalASTMerger.h (+2-1) - (modified) clang/include/clang/AST/ExternalASTSource.h (+9-1) - (modified) clang/include/clang/Sema/MultiplexExternalSemaSource.h (+2-1) - (modified) clang/include/clang/Serialization/ASTReader.h (+2-1) - (modified) clang/lib/AST/DeclBase.cpp (+10-5) - (modified) clang/lib/AST/ExternalASTMerger.cpp (+2-1) - (modified) clang/lib/AST/ExternalASTSource.cpp (+2-1) - (modified) clang/lib/Interpreter/CodeCompletion.cpp (+4-2) - (modified) clang/lib/Sema/MultiplexExternalSemaSource.cpp (+3-2) - (modified) clang/lib/Serialization/ASTReader.cpp (+2-1) - (modified) clang/unittests/AST/ExternalASTSourceTest.cpp (+2-1) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h (+6-4) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp (+3-2) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h (+5-3) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.cpp (+2-1) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h (+2-1) - (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp (+2-1) ``````````diff 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? `````````` </details> https://github.com/llvm/llvm-project/pull/123152 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
