r174576 - Simplify FindExternalVisibleDeclsByName by making it return a bool indicating
Richard Smith
richard-llvm at metafoo.co.uk
Wed Feb 6 19:30:25 PST 2013
Author: rsmith
Date: Wed Feb 6 21:30:24 2013
New Revision: 174576
URL: http://llvm.org/viewvc/llvm-project?rev=174576&view=rev
Log:
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!
Modified:
cfe/trunk/include/clang/AST/ExternalASTSource.h
cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h
cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/AST/ExternalASTSource.cpp
cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
Modified: cfe/trunk/include/clang/AST/ExternalASTSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTSource.h?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ExternalASTSource.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTSource.h Wed Feb 6 21:30:24 2013
@@ -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
Modified: cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h (original)
+++ cfe/trunk/include/clang/Frontend/ChainedIncludesSource.h Wed Feb 6 21:30:24 2013
@@ -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<Decl*> &Result);
Modified: cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h (original)
+++ cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h Wed Feb 6 21:30:24 2013
@@ -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<Decl*> &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();
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Feb 6 21:30:24 2013
@@ -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);
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Wed Feb 6 21:30:24 2013
@@ -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();
Modified: cfe/trunk/lib/AST/ExternalASTSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTSource.cpp?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExternalASTSource.cpp (original)
+++ cfe/trunk/lib/AST/ExternalASTSource.cpp Wed Feb 6 21:30:24 2013
@@ -43,10 +43,10 @@ ExternalASTSource::GetExternalCXXBaseSpe
return 0;
}
-DeclContextLookupResult
+bool
ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
DeclarationName Name) {
- return DeclContext::lookup_result();
+ return false;
}
void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) {
Modified: cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp (original)
+++ cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp Wed Feb 6 21:30:24 2013
@@ -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);
Modified: cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp (original)
+++ cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp Wed Feb 6 21:30:24 2013
@@ -81,19 +81,12 @@ CXXBaseSpecifier *MultiplexExternalSemaS
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){
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=174576&r1=174575&r2=174576&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Feb 6 21:30:24 2013
@@ -5424,14 +5424,13 @@ static ModuleFile *getDefinitiveModuleFi
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<NamedDecl *, 64> Decls;
@@ -5464,7 +5463,7 @@ ASTReader::FindExternalVisibleDeclsByNam
}
++NumVisibleDeclContextsRead;
SetExternalVisibleDeclsForName(DC, Name, Decls);
- return const_cast<DeclContext*>(DC)->lookup(Name);
+ return !Decls.empty();
}
namespace {
More information about the cfe-commits
mailing list