From 9ce12e36abf44b6373b88610f3c73fbbbdee9b36 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 7 Feb 2013 03:30:24 +0000 Subject: [PATCH] Simplify FindExternalVisibleDeclsByName by making it return a bool indicating if it found any decls, rather than returning a list of found decls. This removes a returning-ArrayRef-to-deleted-storage bug from MultiplexExternalSemaSource (in code not exercised by any of the clang binaries), reduces the work required in the found-no-decls case with PCH, and importantly removes the need for DeclContext::lookup to be reentrant. No functionality change intended! llvm-svn: 174576 --- clang/include/clang/AST/ExternalASTSource.h | 15 ++++---- .../include/clang/Frontend/ChainedIncludesSource.h | 4 +-- .../clang/Sema/MultiplexExternalSemaSource.h | 42 ++-------------------- clang/include/clang/Serialization/ASTReader.h | 2 +- clang/lib/AST/DeclBase.cpp | 10 +++++- clang/lib/AST/ExternalASTSource.cpp | 4 +-- clang/lib/Frontend/ChainedIncludesSource.cpp | 2 +- clang/lib/Sema/MultiplexExternalSemaSource.cpp | 17 +++------ clang/lib/Serialization/ASTReader.cpp | 7 ++-- 9 files changed, 32 insertions(+), 71 deletions(-) diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h index 501f8a79..5f95c77 100644 --- a/clang/include/clang/AST/ExternalASTSource.h +++ b/clang/include/clang/AST/ExternalASTSource.h @@ -118,15 +118,14 @@ public: /// The default implementation of this method is a no-op. virtual CXXBaseSpecifier *GetExternalCXXBaseSpecifiers(uint64_t Offset); - /// \brief Finds all declarations with the given name in the - /// given context. + /// \brief 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. /// - /// Generally the final step of this method is either to call - /// SetExternalVisibleDeclsForName or to recursively call lookup on - /// the DeclContext after calling SetExternalVisibleDecls. - /// - /// The default implementation of this method is a no-op. - virtual DeclContextLookupResult + /// The default implementation of this method is a no-op returning \c false. + virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name); /// \brief Ensures that the table of all visible declarations inside this diff --git a/clang/include/clang/Frontend/ChainedIncludesSource.h b/clang/include/clang/Frontend/ChainedIncludesSource.h index d7119e965..e14580e 100644 --- a/clang/include/clang/Frontend/ChainedIncludesSource.h +++ b/clang/include/clang/Frontend/ChainedIncludesSource.h @@ -44,8 +44,8 @@ protected: virtual uint32_t GetNumExternalSelectors(); virtual Stmt *GetExternalDeclStmt(uint64_t Offset); virtual CXXBaseSpecifier *GetExternalCXXBaseSpecifiers(uint64_t Offset); - virtual DeclContextLookupResult - FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name); + virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC, + DeclarationName Name); virtual ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC, bool (*isKindWeWant)(Decl::Kind), SmallVectorImpl &Result); diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h index 32b7276..ff87d05 100644 --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -65,58 +65,30 @@ public: /// \brief Resolve a declaration ID into a declaration, potentially /// building a new declaration. - /// - /// This method only needs to be implemented if the AST source ever - /// passes back decl sets as VisibleDeclaration objects. - /// - /// The default implementation of this method is a no-op. virtual Decl *GetExternalDecl(uint32_t ID); /// \brief Resolve a selector ID into a selector. - /// - /// This operation only needs to be implemented if the AST source - /// returns non-zero for GetNumKnownSelectors(). - /// - /// The default implementation of this method is a no-op. virtual Selector GetExternalSelector(uint32_t ID); /// \brief Returns the number of selectors known to the external AST /// source. - /// - /// The default implementation of this method is a no-op. virtual uint32_t GetNumExternalSelectors(); /// \brief Resolve the offset of a statement in the decl stream into /// a statement. - /// - /// This operation is meant to be used via a LazyOffsetPtr. It only - /// needs to be implemented if the AST source uses methods like - /// FunctionDecl::setLazyBody when building decls. - /// - /// The default implementation of this method is a no-op. virtual Stmt *GetExternalDeclStmt(uint64_t Offset); /// \brief Resolve the offset of a set of C++ base specifiers in the decl /// stream into an array of specifiers. - /// - /// The default implementation of this method is a no-op. virtual CXXBaseSpecifier *GetExternalCXXBaseSpecifiers(uint64_t Offset); - /// \brief Finds all declarations with the given name in the + /// \brief Find all declarations with the given name in the /// given context. - /// - /// Generally the final step of this method is either to call - /// SetExternalVisibleDeclsForName or to recursively call lookup on - /// the DeclContext after calling SetExternalVisibleDecls. - /// - /// The default implementation of this method is a no-op. - virtual DeclContextLookupResult + virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name); /// \brief Ensures that the table of all visible declarations inside this /// context is up to date. - /// - /// The default implementation of this functino is a no-op. virtual void completeVisibleDeclsMap(const DeclContext *DC); /// \brief Finds all declarations lexically contained within the given @@ -127,8 +99,6 @@ public: /// are returned. /// /// \return an indication of whether the load succeeded or failed. - /// - /// The default implementation of this method is a no-op. virtual ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC, bool (*isKindWeWant)(Decl::Kind), SmallVectorImpl &Result); @@ -172,26 +142,18 @@ public: /// \brief Notify ExternalASTSource that we started deserialization of /// a decl or type so until FinishedDeserializing is called there may be /// decls that are initializing. Must be paired with FinishedDeserializing. - /// - /// The default implementation of this method is a no-op. virtual void StartedDeserializing(); /// \brief Notify ExternalASTSource that we finished the deserialization of /// a decl or type. Must be paired with StartedDeserializing. - /// - /// The default implementation of this method is a no-op. virtual void FinishedDeserializing(); /// \brief Function that will be invoked when we begin parsing a new /// translation unit involving this external AST source. - /// - /// The default implementation of this method is a no-op. virtual void StartTranslationUnit(ASTConsumer *Consumer); /// \brief Print any statistics that have been gathered regarding /// the external AST source. - /// - /// The default implementation of this method is a no-op. virtual void PrintStats(); diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index bfa3994e..ecb550e 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1437,7 +1437,7 @@ public: /// \brief Finds all the visible declarations with a given name. /// The current implementation of this method just loads the entire /// lookup table as unmaterialized references. - virtual DeclContext::lookup_result + virtual bool FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name); diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 27a91cb..52ecdeb 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1186,7 +1186,15 @@ DeclContext::lookup(DeclarationName Name) { } ExternalASTSource *Source = getParentASTContext().getExternalSource(); - return Source->FindExternalVisibleDeclsByName(this, Name); + if (Source->FindExternalVisibleDeclsByName(this, Name)) { + if (StoredDeclsMap *Map = LookupPtr.getPointer()) { + StoredDeclsMap::iterator I = Map->find(Name); + if (I != Map->end()) + return I->second.getLookupResult(); + } + } + + return lookup_result(lookup_iterator(0), lookup_iterator(0)); } StoredDeclsMap *Map = LookupPtr.getPointer(); diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp index 6b9fe26..96ebe92 100644 --- a/clang/lib/AST/ExternalASTSource.cpp +++ b/clang/lib/AST/ExternalASTSource.cpp @@ -43,10 +43,10 @@ ExternalASTSource::GetExternalCXXBaseSpecifiers(uint64_t Offset) { return 0; } -DeclContextLookupResult +bool ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) { - return DeclContext::lookup_result(); + return false; } void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) { diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp b/clang/lib/Frontend/ChainedIncludesSource.cpp index 16eb889..f414f93 100644 --- a/clang/lib/Frontend/ChainedIncludesSource.cpp +++ b/clang/lib/Frontend/ChainedIncludesSource.cpp @@ -191,7 +191,7 @@ CXXBaseSpecifier * ChainedIncludesSource::GetExternalCXXBaseSpecifiers(uint64_t Offset) { return getFinalReader().GetExternalCXXBaseSpecifiers(Offset); } -DeclContextLookupResult +bool ChainedIncludesSource::FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) { return getFinalReader().FindExternalVisibleDeclsByName(DC, Name); diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp index ecce6d8..d85624b 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -81,19 +81,12 @@ CXXBaseSpecifier *MultiplexExternalSemaSource::GetExternalCXXBaseSpecifiers( return 0; } -DeclContextLookupResult MultiplexExternalSemaSource:: +bool MultiplexExternalSemaSource:: FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) { - StoredDeclsList DeclsFound; - for(size_t i = 0; i < Sources.size(); ++i) { - DeclContext::lookup_result R = - Sources[i]->FindExternalVisibleDeclsByName(DC, Name); - for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; - ++I) { - if (!DeclsFound.HandleRedeclaration(*I)) - DeclsFound.AddSubsequentDecl(*I); - } - } - return DeclsFound.getLookupResult(); + bool AnyDeclsFound = false; + for (size_t i = 0; i < Sources.size(); ++i) + AnyDeclsFound |= Sources[i]->FindExternalVisibleDeclsByName(DC, Name); + return AnyDeclsFound; } void MultiplexExternalSemaSource::completeVisibleDeclsMap(const DeclContext *DC){ diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index aa4c448..1213084 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5424,14 +5424,13 @@ static ModuleFile *getDefinitiveModuleFileFor(const DeclContext *DC, return 0; } -DeclContext::lookup_result +bool ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) { assert(DC->hasExternalVisibleStorage() && "DeclContext has no visible decls in storage"); if (!Name) - return DeclContext::lookup_result(DeclContext::lookup_iterator(0), - DeclContext::lookup_iterator(0)); + return false; SmallVector Decls; @@ -5464,7 +5463,7 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC, } ++NumVisibleDeclContextsRead; SetExternalVisibleDeclsForName(DC, Name, Decls); - return const_cast(DC)->lookup(Name); + return !Decls.empty(); } namespace { -- 2.7.4