r244161 - function_ref-ize ExternalASTSource::FindExternalLexicalDecl and remove its

Richard Smith richard-llvm at metafoo.co.uk
Wed Aug 5 15:41:45 PDT 2015


Author: rsmith
Date: Wed Aug  5 17:41:45 2015
New Revision: 244161

URL: http://llvm.org/viewvc/llvm-project?rev=244161&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/AST/ExternalASTSource.h
    cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/include/clang/Serialization/ModuleManager.h
    cfe/trunk/lib/AST/Decl.cpp
    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=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ExternalASTSource.h (original)
+++ cfe/trunk/include/clang/AST/ExternalASTSource.h Wed Aug  5 17:41:45 2015
@@ -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

Modified: cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h (original)
+++ cfe/trunk/include/clang/Sema/MultiplexExternalSemaSource.h Wed Aug  5 17:41:45 2015
@@ -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

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Wed Aug  5 17:41:45 2015
@@ -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

Modified: cfe/trunk/include/clang/Serialization/ModuleManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ModuleManager.h?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ModuleManager.h (original)
+++ cfe/trunk/include/clang/Serialization/ModuleManager.h Wed Aug  5 17:41:45 2015
@@ -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]; }

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Wed Aug  5 17:41:45 2015
@@ -3639,10 +3639,6 @@ bool RecordDecl::isMsStruct(const ASTCon
   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::LoadFieldsFromExternalS
   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.

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Wed Aug  5 17:41:45 2015
@@ -1059,14 +1059,7 @@ DeclContext::LoadLexicalDeclsFromExterna
   // 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;

Modified: cfe/trunk/lib/AST/ExternalASTSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExternalASTSource.cpp?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExternalASTSource.cpp (original)
+++ cfe/trunk/lib/AST/ExternalASTSource.cpp Wed Aug  5 17:41:45 2015
@@ -92,17 +92,13 @@ ExternalASTSource::FindExternalVisibleDe
   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;

Modified: cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp (original)
+++ cfe/trunk/lib/Frontend/ChainedIncludesSource.cpp Wed Aug  5 17:41:45 2015
@@ -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::FindExternalVisib
                                                       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);

Modified: cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp (original)
+++ cfe/trunk/lib/Sema/MultiplexExternalSemaSource.cpp Wed Aug  5 17:41:45 2015
@@ -107,15 +107,11 @@ void MultiplexExternalSemaSource::comple
     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, 

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=244161&r1=244160&r2=244161&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Aug  5 17:41:45 2015
@@ -5910,8 +5910,13 @@ void ASTReader::CompleteRedeclChain(cons
       } 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 {




More information about the cfe-commits mailing list