From b5cef75c82aa309ac594c31b33259ffa680fc95d Mon Sep 17 00:00:00 2001
From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Wed, 5 Aug 2015 22:41:45 +0000
Subject: [PATCH] function_ref-ize ExternalASTSource::FindExternalLexicalDecl
 and remove its useless return value. Switch to using it directly when
 completing the redeclaration chain for an anonymous declaration, and reduce
 the set of declarations that we load in the process to just those of the
 right kind.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@244161 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/clang/AST/ExternalASTSource.h         | 42 ++++-----------
 .../clang/Sema/MultiplexExternalSemaSource.h  | 29 +++-------
 include/clang/Serialization/ASTReader.h       | 13 ++---
 include/clang/Serialization/ModuleManager.h   | 11 ++--
 lib/AST/Decl.cpp                              | 18 ++-----
 lib/AST/DeclBase.cpp                          |  9 +---
 lib/AST/ExternalASTSource.cpp                 | 14 ++---
 lib/Frontend/ChainedIncludesSource.cpp        | 13 +++--
 lib/Sema/MultiplexExternalSemaSource.cpp      | 12 ++---
 lib/Serialization/ASTReader.cpp               | 54 ++++++++++---------
 10 files changed, 76 insertions(+), 139 deletions(-)

diff --git a/include/clang/AST/ExternalASTSource.h b/include/clang/AST/ExternalASTSource.h
index 08c2e0cf267..d5d52e60b1f 100644
--- a/include/clang/AST/ExternalASTSource.h
+++ b/include/clang/AST/ExternalASTSource.h
@@ -33,20 +33,6 @@ class Selector;
 class Stmt;
 class TagDecl;
 
-/// \brief Enumeration describing the result of loading information from
-/// an external source.
-enum ExternalLoadResult {
-  /// \brief Loading the external information has succeeded.
-  ELR_Success,
-  
-  /// \brief Loading the external information has failed.
-  ELR_Failure,
-  
-  /// \brief The external information has already been loaded, and therefore
-  /// no additional processing is required.
-  ELR_AlreadyLoaded
-};
-
 /// \brief Abstract interface for external sources of AST nodes.
 ///
 /// External AST sources provide AST nodes constructed from some
@@ -173,30 +159,20 @@ public:
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.
   ///
-  /// \param isKindWeWant a predicate function that returns true if the passed
-  /// declaration kind is one we are looking for. If NULL, all declarations
-  /// are returned.
-  ///
-  /// \return an indication of whether the load succeeded or failed.
+  /// \param IsKindWeWant a predicate function that returns true if the passed
+  /// declaration kind is one we are looking for.
   ///
   /// The default implementation of this method is a no-op.
-  virtual ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC,
-                                        bool (*isKindWeWant)(Decl::Kind),
-                                        SmallVectorImpl<Decl*> &Result);
+  virtual void
+  FindExternalLexicalDecls(const DeclContext *DC,
+                           llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+                           SmallVectorImpl<Decl *> &Result);
 
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext.
-  ///
-  /// \return true if an error occurred
-  ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC,
-                                SmallVectorImpl<Decl*> &Result) {
-    return FindExternalLexicalDecls(DC, nullptr, Result);
-  }
-
-  template <typename DeclTy>
-  ExternalLoadResult FindExternalLexicalDeclsBy(const DeclContext *DC,
-                                  SmallVectorImpl<Decl*> &Result) {
-    return FindExternalLexicalDecls(DC, DeclTy::classofKind, Result);
+  void FindExternalLexicalDecls(const DeclContext *DC,
+                                SmallVectorImpl<Decl *> &Result) {
+    FindExternalLexicalDecls(DC, [](Decl::Kind) { return true; }, Result);
   }
 
   /// \brief Get the decls that are contained in a file in the Offset/Length
diff --git a/include/clang/Sema/MultiplexExternalSemaSource.h b/include/clang/Sema/MultiplexExternalSemaSource.h
index af7083a82de..d6daadc89da 100644
--- a/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -102,29 +102,12 @@ public:
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.
   ///
-  /// \param isKindWeWant a predicate function that returns true if the passed
-  /// declaration kind is one we are looking for. If NULL, all declarations
-  /// are returned.
-  ///
-  /// \return an indication of whether the load succeeded or failed.
-  ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC,
-                                bool (*isKindWeWant)(Decl::Kind),
-                                SmallVectorImpl<Decl*> &Result) override;
-
-  /// \brief Finds all declarations lexically contained within the given
-  /// DeclContext.
-  ///
-  /// \return true if an error occurred
-  ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC,
-                                SmallVectorImpl<Decl*> &Result) {
-    return FindExternalLexicalDecls(DC, nullptr, Result);
-  }
-
-  template <typename DeclTy>
-  ExternalLoadResult FindExternalLexicalDeclsBy(const DeclContext *DC,
-                                  SmallVectorImpl<Decl*> &Result) {
-    return FindExternalLexicalDecls(DC, DeclTy::classofKind, Result);
-  }
+  /// \param IsKindWeWant a predicate function that returns true if the passed
+  /// declaration kind is one we are looking for.
+  void
+  FindExternalLexicalDecls(const DeclContext *DC,
+                           llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+                           SmallVectorImpl<Decl *> &Result) override;
 
   /// \brief Get the decls that are contained in a file in the Offset/Length
   /// range. \p Length can be 0 to indicate a point at \p Offset instead of
diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 029b3570c58..de6f43f8513 100644
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -1698,16 +1698,17 @@ public:
   /// \param DC The declaration context whose declarations will be
   /// read.
   ///
+  /// \param IsKindWeWant A predicate indicating which declaration kinds
+  /// we are interested in.
+  ///
   /// \param Decls Vector that will contain the declarations loaded
   /// from the external source. The caller is responsible for merging
   /// these declarations with any declarations already stored in the
   /// declaration context.
-  ///
-  /// \returns true if there was an error while reading the
-  /// declarations for this declaration context.
-  ExternalLoadResult FindExternalLexicalDecls(const DeclContext *DC,
-                                bool (*isKindWeWant)(Decl::Kind),
-                                SmallVectorImpl<Decl*> &Decls) override;
+  void
+  FindExternalLexicalDecls(const DeclContext *DC,
+                           llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+                           SmallVectorImpl<Decl *> &Decls) override;
 
   /// \brief Get the decls that are contained in a file in the Offset/Length
   /// range. \p Length can be 0 to indicate a point at \p Offset instead of
diff --git a/include/clang/Serialization/ModuleManager.h b/include/clang/Serialization/ModuleManager.h
index bf3525a2504..b191a1dbb87 100644
--- a/include/clang/Serialization/ModuleManager.h
+++ b/include/clang/Serialization/ModuleManager.h
@@ -126,20 +126,17 @@ public:
                          const PCHContainerReader &PCHContainerRdr);
   ~ModuleManager();
 
-  /// \brief Forward iterator to traverse all loaded modules.  This is reverse
-  /// source-order.
+  /// \brief Forward iterator to traverse all loaded modules.
   ModuleIterator begin() { return Chain.begin(); }
   /// \brief Forward iterator end-point to traverse all loaded modules
   ModuleIterator end() { return Chain.end(); }
   
-  /// \brief Const forward iterator to traverse all loaded modules.  This is 
-  /// in reverse source-order.
+  /// \brief Const forward iterator to traverse all loaded modules.
   ModuleConstIterator begin() const { return Chain.begin(); }
   /// \brief Const forward iterator end-point to traverse all loaded modules
   ModuleConstIterator end() const { return Chain.end(); }
   
-  /// \brief Reverse iterator to traverse all loaded modules.  This is in 
-  /// source order.
+  /// \brief Reverse iterator to traverse all loaded modules.
   ModuleReverseIterator rbegin() { return Chain.rbegin(); }
   /// \brief Reverse iterator end-point to traverse all loaded modules.
   ModuleReverseIterator rend() { return Chain.rend(); }
@@ -148,7 +145,7 @@ public:
   llvm::iterator_range<ModuleConstIterator> pch_modules() const {
     return llvm::make_range(PCHChain.begin(), PCHChain.end());
   }
-  
+
   /// \brief Returns the primary module associated with the manager, that is,
   /// the first module loaded
   ModuleFile &getPrimaryModule() { return *Chain[0]; }
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 9d44a69e91d..ab96094ba82 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -3639,10 +3639,6 @@ bool RecordDecl::isMsStruct(const ASTContext &C) const {
   return hasAttr<MSStructAttr>() || C.getLangOpts().MSBitfields == 1;
 }
 
-static bool isFieldOrIndirectField(Decl::Kind K) {
-  return FieldDecl::classofKind(K) || IndirectFieldDecl::classofKind(K);
-}
-
 void RecordDecl::LoadFieldsFromExternalStorage() const {
   ExternalASTSource *Source = getASTContext().getExternalSource();
   assert(hasExternalLexicalStorage() && Source && "No external storage?");
@@ -3651,16 +3647,10 @@ void RecordDecl::LoadFieldsFromExternalStorage() const {
   ExternalASTSource::Deserializing TheFields(Source);
 
   SmallVector<Decl*, 64> Decls;
-  LoadedFieldsFromExternalStorage = true;  
-  switch (Source->FindExternalLexicalDecls(this, isFieldOrIndirectField,
-                                           Decls)) {
-  case ELR_Success:
-    break;
-    
-  case ELR_AlreadyLoaded:
-  case ELR_Failure:
-    return;
-  }
+  LoadedFieldsFromExternalStorage = true;
+  Source->FindExternalLexicalDecls(this, [](Decl::Kind K) {
+    return FieldDecl::classofKind(K) || IndirectFieldDecl::classofKind(K);
+  }, Decls);
 
 #ifndef NDEBUG
   // Check that all decls we got were FieldDecls.
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index e772f195336..40b135f9202 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -1059,14 +1059,7 @@ DeclContext::LoadLexicalDeclsFromExternalStorage() const {
   // Load the external declarations, if any.
   SmallVector<Decl*, 64> Decls;
   ExternalLexicalStorage = false;
-  switch (Source->FindExternalLexicalDecls(this, Decls)) {
-  case ELR_Success:
-    break;
-    
-  case ELR_Failure:
-  case ELR_AlreadyLoaded:
-    return false;
-  }
+  Source->FindExternalLexicalDecls(this, Decls);
 
   if (Decls.empty())
     return false;
diff --git a/lib/AST/ExternalASTSource.cpp b/lib/AST/ExternalASTSource.cpp
index 1c82c355134..8a44c05dbb7 100644
--- a/lib/AST/ExternalASTSource.cpp
+++ b/lib/AST/ExternalASTSource.cpp
@@ -92,17 +92,13 @@ ExternalASTSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
   return false;
 }
 
-void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) {
-}
+void ExternalASTSource::completeVisibleDeclsMap(const DeclContext *DC) {}
 
-ExternalLoadResult
-ExternalASTSource::FindExternalLexicalDecls(const DeclContext *DC,
-                                            bool (*isKindWeWant)(Decl::Kind),
-                                         SmallVectorImpl<Decl*> &Result) {
-  return ELR_AlreadyLoaded;
-}
+void ExternalASTSource::FindExternalLexicalDecls(
+    const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+    SmallVectorImpl<Decl *> &Result) {}
 
-void ExternalASTSource::getMemoryBufferSizes(MemoryBufferSizes &sizes) const { }
+void ExternalASTSource::getMemoryBufferSizes(MemoryBufferSizes &sizes) const {}
 
 uint32_t ExternalASTSource::incrementGeneration(ASTContext &C) {
   uint32_t OldGeneration = CurrentGeneration;
diff --git a/lib/Frontend/ChainedIncludesSource.cpp b/lib/Frontend/ChainedIncludesSource.cpp
index cc0504ba8b2..5a5d2d55eda 100644
--- a/lib/Frontend/ChainedIncludesSource.cpp
+++ b/lib/Frontend/ChainedIncludesSource.cpp
@@ -47,9 +47,9 @@ protected:
   CXXBaseSpecifier *GetExternalCXXBaseSpecifiers(uint64_t Offset) override;
   bool FindExternalVisibleDeclsByName(const DeclContext *DC,
                                       DeclarationName Name) override;
-  ExternalLoadResult
+  void
   FindExternalLexicalDecls(const DeclContext *DC,
-                           bool (*isKindWeWant)(Decl::Kind),
+                           llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
                            SmallVectorImpl<Decl *> &Result) override;
   void CompleteType(TagDecl *Tag) override;
   void CompleteType(ObjCInterfaceDecl *Class) override;
@@ -246,11 +246,10 @@ ChainedIncludesSource::FindExternalVisibleDeclsByName(const DeclContext *DC,
                                                       DeclarationName Name) {
   return getFinalReader().FindExternalVisibleDeclsByName(DC, Name);
 }
-ExternalLoadResult 
-ChainedIncludesSource::FindExternalLexicalDecls(const DeclContext *DC,
-                                      bool (*isKindWeWant)(Decl::Kind),
-                                      SmallVectorImpl<Decl*> &Result) {
-  return getFinalReader().FindExternalLexicalDecls(DC, isKindWeWant, Result);
+void ChainedIncludesSource::FindExternalLexicalDecls(
+    const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+    SmallVectorImpl<Decl *> &Result) {
+  return getFinalReader().FindExternalLexicalDecls(DC, IsKindWeWant, Result);
 }
 void ChainedIncludesSource::CompleteType(TagDecl *Tag) {
   return getFinalReader().CompleteType(Tag);
diff --git a/lib/Sema/MultiplexExternalSemaSource.cpp b/lib/Sema/MultiplexExternalSemaSource.cpp
index 9ecb5a7fefb..0f93421ac21 100644
--- a/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -107,15 +107,11 @@ void MultiplexExternalSemaSource::completeVisibleDeclsMap(const DeclContext *DC)
     Sources[i]->completeVisibleDeclsMap(DC);
 }
 
-ExternalLoadResult MultiplexExternalSemaSource::
-FindExternalLexicalDecls(const DeclContext *DC,
-                         bool (*isKindWeWant)(Decl::Kind),
-                         SmallVectorImpl<Decl*> &Result) {
+void MultiplexExternalSemaSource::FindExternalLexicalDecls(
+    const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+    SmallVectorImpl<Decl *> &Result) {
   for(size_t i = 0; i < Sources.size(); ++i)
-    // FIXME: The semantics of the return result is unclear to me...
-    Sources[i]->FindExternalLexicalDecls(DC, isKindWeWant, Result);
-
-  return ELR_Success;
+    Sources[i]->FindExternalLexicalDecls(DC, IsKindWeWant, Result);
 }
 
 void MultiplexExternalSemaSource::FindFileRegionDecls(FileID File, 
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 52231387ef2..abc91e771ec 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -5910,8 +5910,13 @@ void ASTReader::CompleteRedeclChain(const Decl *D) {
       } else
         DC->lookup(Name);
     } else if (needsAnonymousDeclarationNumber(cast<NamedDecl>(D))) {
-      // FIXME: It'd be nice to do something a bit more targeted here.
-      D->getDeclContext()->decls_begin();
+      // Find all declarations of this kind from the relevant context.
+      for (auto *DCDecl : cast<Decl>(D->getLexicalDeclContext())->redecls()) {
+        auto *DC = cast<DeclContext>(DCDecl);
+        SmallVector<Decl*, 8> Decls;
+        FindExternalLexicalDecls(
+            DC, [&](Decl::Kind K) { return K == D->getKind(); }, Decls);
+      }
     }
   }
 
@@ -6174,47 +6179,48 @@ namespace {
   class FindExternalLexicalDeclsVisitor {
     ASTReader &Reader;
     const DeclContext *DC;
-    bool (*isKindWeWant)(Decl::Kind);
+    llvm::function_ref<bool(Decl::Kind)> IsKindWeWant;
     
     SmallVectorImpl<Decl*> &Decls;
     bool PredefsVisited[NUM_PREDEF_DECL_IDS];
 
   public:
-    FindExternalLexicalDeclsVisitor(ASTReader &Reader, const DeclContext *DC,
-                                    bool (*isKindWeWant)(Decl::Kind),
-                                    SmallVectorImpl<Decl*> &Decls)
-      : Reader(Reader), DC(DC), isKindWeWant(isKindWeWant), Decls(Decls) 
-    {
+    FindExternalLexicalDeclsVisitor(
+        ASTReader &Reader, const DeclContext *DC,
+        llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+        SmallVectorImpl<Decl *> &Decls)
+        : Reader(Reader), DC(DC), IsKindWeWant(IsKindWeWant), Decls(Decls) {
       for (unsigned I = 0; I != NUM_PREDEF_DECL_IDS; ++I)
         PredefsVisited[I] = false;
     }
 
     static bool visitPostorder(ModuleFile &M, void *UserData) {
-      FindExternalLexicalDeclsVisitor *This
-        = static_cast<FindExternalLexicalDeclsVisitor *>(UserData);
+      return (*static_cast<FindExternalLexicalDeclsVisitor*>(UserData))(M);
+    }
 
-      ModuleFile::DeclContextInfosMap::iterator Info
-        = M.DeclContextInfos.find(This->DC);
+    bool operator()(ModuleFile &M) {
+      ModuleFile::DeclContextInfosMap::iterator Info =
+          M.DeclContextInfos.find(DC);
       if (Info == M.DeclContextInfos.end() || Info->second.LexicalDecls.empty())
         return false;
 
       // Load all of the declaration IDs
       for (const KindDeclIDPair &P : Info->second.LexicalDecls) {
-        if (This->isKindWeWant && !This->isKindWeWant((Decl::Kind)P.first))
+        if (!IsKindWeWant((Decl::Kind)P.first))
           continue;
 
         // Don't add predefined declarations to the lexical context more
         // than once.
         if (P.second < NUM_PREDEF_DECL_IDS) {
-          if (This->PredefsVisited[P.second])
+          if (PredefsVisited[P.second])
             continue;
 
-          This->PredefsVisited[P.second] = true;
+          PredefsVisited[P.second] = true;
         }
 
-        if (Decl *D = This->Reader.GetLocalDecl(M, P.second)) {
-          if (!This->DC->isDeclInLexicalTraversal(D))
-            This->Decls.push_back(D);
+        if (Decl *D = Reader.GetLocalDecl(M, P.second)) {
+          if (!DC->isDeclInLexicalTraversal(D))
+            Decls.push_back(D);
         }
       }
 
@@ -6223,16 +6229,16 @@ namespace {
   };
 }
 
-ExternalLoadResult ASTReader::FindExternalLexicalDecls(const DeclContext *DC,
-                                         bool (*isKindWeWant)(Decl::Kind),
-                                         SmallVectorImpl<Decl*> &Decls) {
+void ASTReader::FindExternalLexicalDecls(
+    const DeclContext *DC, llvm::function_ref<bool(Decl::Kind)> IsKindWeWant,
+    SmallVectorImpl<Decl *> &Decls) {
   // There might be lexical decls in multiple modules, for the TU at
-  // least. Walk all of the modules in the order they were loaded.
-  FindExternalLexicalDeclsVisitor Visitor(*this, DC, isKindWeWant, Decls);
+  // least. FIXME: Only look in multiple module files in the very rare
+  // cases where this can actually happen.
+  FindExternalLexicalDeclsVisitor Visitor(*this, DC, IsKindWeWant, Decls);
   ModuleMgr.visitDepthFirst(
       nullptr, &FindExternalLexicalDeclsVisitor::visitPostorder, &Visitor);
   ++NumLexicalDeclContextsRead;
-  return ELR_Success;
 }
 
 namespace {
-- 
GitLab