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!


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@174576 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 32 insertions, 71 deletions

diff --git a/include/clang/AST/ExternalASTSource.h b/include/clang/AST/ExternalASTSource.h
index 501f8a796d..5f95c775dc 100644
--- a/include/clang/AST/ExternalASTSource.h
+++ b/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/include/clang/Frontend/ChainedIncludesSource.h b/include/clang/Frontend/ChainedIncludesSource.h
index d7119e9653..e14580ed69 100644
--- a/include/clang/Frontend/ChainedIncludesSource.h
+++ b/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<Decl*> &Result);
diff --git a/include/clang/Sema/MultiplexExternalSemaSource.h b/include/clang/Sema/MultiplexExternalSemaSource.h
index 32b7276c83..ff87d05c04 100644
--- a/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/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<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();
   
   
diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index bfa3994ec3..ecb550e26b 100644
--- a/include/clang/Serialization/ASTReader.h
+++ b/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/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 27a91cb7e6..52ecdeb318 100644
--- a/lib/AST/DeclBase.cpp
+++ b/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/lib/AST/ExternalASTSource.cpp b/lib/AST/ExternalASTSource.cpp
index 6b9fe26ccc..96ebe92ce3 100644
--- a/lib/AST/ExternalASTSource.cpp
+++ b/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/lib/Frontend/ChainedIncludesSource.cpp b/lib/Frontend/ChainedIncludesSource.cpp
index 16eb8890d9..f414f93812 100644
--- a/lib/Frontend/ChainedIncludesSource.cpp
+++ b/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/lib/Sema/MultiplexExternalSemaSource.cpp b/lib/Sema/MultiplexExternalSemaSource.cpp
index ecce6d8e7b..d85624ba6f 100644
--- a/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/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/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index aa4c448e98..1213084e36 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/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<NamedDecl *, 64> Decls;
   
@@ -5464,7 +5463,7 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
   }
   ++NumVisibleDeclContextsRead;
   SetExternalVisibleDeclsForName(DC, Name, Decls);
-  return const_cast<DeclContext*>(DC)->lookup(Name);
+  return !Decls.empty();
 }
 
 namespace {