Fix ASTReader::FinishedDeserializing().

We were passing a decl to the consumer after all pending deserializations were finished
but this was not enough; due to processing by the consumer we may end up into yet another
deserialization process but the way FinishedDeserializing() was setup we would not ensure
that everything was fully deserialized before returning to the consumer.

Separate ASTReader::FinishedDeserializing() into two semantic actions.
The first is ensuring that a deserialization process ends up will fully deserialized decls/types even
if the process is started by the consumer.
The second is pushing "interesting" decls to the consumer; we make sure that we don't re-enter this
section recursively be checking a variable.

llvm-svn: 150160

clang/include/clang/Serialization/ASTReader.h

@@ -639,6 +639,12 @@ private:
   /// \brief Number of Decl/types that are currently deserializing.
   unsigned NumCurrentElementsDeserializing;
 
+  /// \brief Set true while we are in the process of passing deserialized
+  /// "interesting" decls to consumer inside FinishedDeserializing().
+  /// This is used as a guard to avoid recursively repeating the process of
+  /// passing decls to consumer.
+  bool PassingDeclsToConsumer;
+
   /// Number of CXX base specifiers currently loaded
   unsigned NumCXXBaseSpecifiersLoaded;
 

clang/lib/Serialization/ASTReader.cpp

@@ -27,6 +27,7 @@
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLocVisitor.h"
+#include "clang/Analysis/Support/SaveAndRestore.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/PreprocessingRecord.h"
 #include "clang/Lex/Preprocessor.h"
@@ -6246,33 +6247,27 @@ void ASTReader::FinishedDeserializing() {
   assert(NumCurrentElementsDeserializing &&
          "FinishedDeserializing not paired with StartedDeserializing");
   if (NumCurrentElementsDeserializing == 1) {
+    // We decrease NumCurrentElementsDeserializing only after pending actions
+    // are finished, to avoid recursively re-calling finishPendingActions().
+    finishPendingActions();
+  }
+  --NumCurrentElementsDeserializing;
 
-    while (Consumer && !InterestingDecls.empty()) {
-      finishPendingActions();
+  if (NumCurrentElementsDeserializing == 0 &&
+      Consumer && !PassingDeclsToConsumer) {
+    // Guard variable to avoid recursively redoing the process of passing
+    // decls to consumer.
+    SaveAndRestore<bool> GuardPassingDeclsToConsumer(PassingDeclsToConsumer,
+                                                     true);
 
+    while (!InterestingDecls.empty()) {
       // We are not in recursive loading, so it's safe to pass the "interesting"
       // decls to the consumer.
       Decl *D = InterestingDecls.front();
       InterestingDecls.pop_front();
-
-      // Fully load the interesting decls, including deserializing their
-      // bodies, so that any other declarations that get referenced in the
-      // body will be fully deserialized by the time we pass them to the
-      // consumer.
-      if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
-        if (FD->doesThisDeclarationHaveABody()) {
-          FD->getBody();
-          finishPendingActions();
-        }
-      }
-
       PassInterestingDeclToConsumer(D);
     }
-
-    finishPendingActions();
-    PendingDeclChainsKnown.clear();
   }
-  --NumCurrentElementsDeserializing;
 }
 
 ASTReader::ASTReader(Preprocessor &PP, ASTContext &Context,
@@ -6293,6 +6288,7 @@ ASTReader::ASTReader(Preprocessor &PP, ASTContext &Context,
     NumLexicalDeclContextsRead(0), TotalLexicalDeclContexts(0), 
     NumVisibleDeclContextsRead(0), TotalVisibleDeclContexts(0),
     TotalModulesSizeInBits(0), NumCurrentElementsDeserializing(0),
+    PassingDeclsToConsumer(false),
     NumCXXBaseSpecifiersLoaded(0)
 {
   SourceMgr.setExternalSLocEntrySource(this);