Remove PendingBody mechanism for function and ObjC method deserialization.

In its place, track on the canonical function declaration whether there is a
declaration with a body (and if so, which one). This brings function definition
handling in line with what we do in all other contexts, and is necessary to
allow us to merge declarations within multiple definitions of the same function
(eg, PR33924).

No functionality change intended.

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

diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 9c5ad00691c518ef271f8d602f36ff1f6532e49c..d6ae9d3a22058e1e3df92a75fb488934bc65691d 100644 (file)
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -559,13 +559,9 @@ private:
   /// declarations that have not yet been linked to their definitions.
   llvm::SmallPtrSet<Decl *, 4> PendingDefinitions;
 
-  typedef llvm::MapVector<Decl *, uint64_t,
-                          llvm::SmallDenseMap<Decl *, unsigned, 4>,
-                          SmallVector<std::pair<Decl *, uint64_t>, 4> >
-    PendingBodiesMap;
-
-  /// \brief Functions or methods that have bodies that will be attached.
-  PendingBodiesMap PendingBodies;
+  /// \brief Functions or methods that are known to already have a definition
+  /// (that might not yet be merged into the redeclaration chain).
+  llvm::SmallDenseMap<FunctionDecl *, FunctionDecl*, 4> FunctionDefinitions;
 
   /// \brief Definitions for which we have added merged definitions but not yet
   /// performed deduplication.
@@ -991,25 +987,13 @@ private:
   /// the last time we loaded information about this identifier.
   llvm::DenseMap<IdentifierInfo *, unsigned> IdentifierGeneration;
 
-  class InterestingDecl {
-    Decl *D;
-    bool DeclHasPendingBody;
-
-  public:
-    InterestingDecl(Decl *D, bool HasBody)
-        : D(D), DeclHasPendingBody(HasBody) {}
-    Decl *getDecl() { return D; }
-    /// Whether the declaration has a pending body.
-    bool hasPendingBody() { return DeclHasPendingBody; }
-  };
-
   /// \brief Contains declarations and definitions that could be
   /// "interesting" to the ASTConsumer, when we get that AST consumer.
   ///
   /// "Interesting" declarations are those that have data that may
   /// need to be emitted, such as inline function definitions or
   /// Objective-C protocols.
-  std::deque<InterestingDecl> PotentiallyInterestingDecls;
+  std::deque<Decl*> PotentiallyInterestingDecls;
 
   /// \brief The list of redeclaration chains that still need to be 
   /// reconstructed, and the local offset to the corresponding list

diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index 4ac8aa3855853f8bbe0a203a315f2de0b838a0d6..29051ebeefcab08693aa8b2bbf21e290fd528b8c 100644 (file)
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -9168,30 +9168,6 @@ void ASTReader::finishPendingActions() {
   }
   PendingDefinitions.clear();
 
-  // Load the bodies of any functions or methods we've encountered. We do
-  // this now (delayed) so that we can be sure that the declaration chains
-  // have been fully wired up (hasBody relies on this).
-  // FIXME: We shouldn't require complete redeclaration chains here.
-  for (PendingBodiesMap::iterator PB = PendingBodies.begin(),
-                               PBEnd = PendingBodies.end();
-       PB != PBEnd; ++PB) {
-    if (FunctionDecl *FD = dyn_cast<FunctionDecl>(PB->first)) {
-      // FIXME: Check for =delete/=default?
-      // FIXME: Complain about ODR violations here?
-      const FunctionDecl *Defn = nullptr;
-      if (!getContext().getLangOpts().Modules || !FD->hasBody(Defn)) {
-        FD->setLazyBody(PB->second);
-      } else
-        mergeDefinitionVisibility(const_cast<FunctionDecl*>(Defn), FD);
-      continue;
-    }
-
-    ObjCMethodDecl *MD = cast<ObjCMethodDecl>(PB->first);
-    if (!getContext().getLangOpts().Modules || !MD->hasBody())
-      MD->setLazyBody(PB->second);
-  }
-  PendingBodies.clear();
-
   // Do some cleanup.
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
     getContext().deduplicateMergedDefinitonsFor(ND);

diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index 1283b006dfb305b0dfd964d4f14c61e939ae9881..57f509ea6f694430e9ea61dd87b3a5451826345d 100644 (file)
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/lib/Serialization/ASTReaderDecl.cpp
@@ -45,8 +45,6 @@ namespace clang {
     GlobalDeclID NamedDeclForTagDecl;
     IdentifierInfo *TypedefNameForLinkage;
 
-    bool HasPendingBody;
-
     ///\brief A flag to carry the information for a decl from the entity is
     /// used. We use it to delay the marking of the canonical decl as used until
     /// the entire declaration is deserialized and merged.
@@ -216,8 +214,7 @@ namespace clang {
         : Reader(Reader), Record(Record), Loc(Loc),
           ThisDeclID(thisDeclID), ThisDeclLoc(ThisDeclLoc),
           TypeIDForTypeDecl(0), NamedDeclForTagDecl(0),
-          TypedefNameForLinkage(nullptr), HasPendingBody(false),
-          IsDeclMarkedUsed(false) {}
+          TypedefNameForLinkage(nullptr), IsDeclMarkedUsed(false) {}
 
     template <typename T> static
     void AddLazySpecializations(T *D,
@@ -265,9 +262,7 @@ namespace clang {
     static void markIncompleteDeclChainImpl(Redeclarable<DeclT> *D);
     static void markIncompleteDeclChainImpl(...);
 
-    /// \brief Determine whether this declaration has a pending body.
-    bool hasPendingBody() const { return HasPendingBody; }
-
+    FunctionDecl *TryRegisterAsFunctionDefinition(FunctionDecl *FD);
     void ReadFunctionDefinition(FunctionDecl *FD);
     void Visit(Decl *D);
 
@@ -420,7 +415,8 @@ public:
   MergedRedeclIterator &operator++() {
     if (Current->isFirstDecl()) {
       Canonical = Current;
-      Current = Current->getMostRecentDecl();
+      Decl *MostRecent = ASTDeclReader::getMostRecentDecl(Canonical);
+      Current = MostRecent ? cast<DeclT>(MostRecent) : nullptr;
     } else
       Current = Current->getPreviousDecl();
 
@@ -451,17 +447,69 @@ uint64_t ASTDeclReader::GetCurrentCursorOffset() {
   return Loc.F->DeclsCursor.GetCurrentBitNo() + Loc.F->GlobalBitOffset;
 }
 
+FunctionDecl *ASTDeclReader::TryRegisterAsFunctionDefinition(FunctionDecl *D) {
+  FunctionDecl *&Definition = Reader.FunctionDefinitions[D->getCanonicalDecl()];
+
+  if (!Definition) {
+    // No imported definition, but we might have a local definition.
+    for (auto *Redecl : merged_redecls(D)) {
+      // FIXME: If an existing declaration is a definition with no body
+      // (via =delete etc), we shouldn't permit another definition here.
+      if (Redecl->Body.isValid()) {
+        Definition = Redecl;
+        break;
+      }
+    }
+  }
+
+  if (Definition) {
+    // We might have multiple update records adding definitions to the same
+    // declaration.
+    if (Definition != D) {
+      // Already have a different definition, merge this one into it.
+      Reader.mergeDefinitionVisibility(Definition, D);
+    }
+    return Definition;
+  }
+
+  Definition = D;
+  return nullptr;
+}
+
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
-  if (Record.readInt())
+  // Is this definition scheduled for modular codegen? (That is, emitted to a
+  // separate object file for the module itself, rather than with module users.)
+  bool ModularCodegen = Record.readInt();
+
+  // Don't load this definition if we already have one.
+  if (auto *Definition = TryRegisterAsFunctionDefinition(FD)) {
+    if (ModularCodegen) {
+      // Request a strong definition be emitted if any merged definition does.
+      // FIXME: Do we need to ensure that Definition is handed to the AST
+      // consumer in this case?
+      Reader.DefinitionSource[Definition] |=
+          Loc.F->Kind == ModuleKind::MK_MainFile;
+    }
+    // Skip the ctor-initializers, if any.
+    if (isa<CXXConstructorDecl>(FD))
+      if (Record.readInt())
+        ReadGlobalOffset();
+    // FIXME: Optionally register the duplicate definitions somewhere so we can
+    // check for ODR violations.
+    return;
+  }
+
+  if (ModularCodegen)
     Reader.DefinitionSource[FD] = Loc.F->Kind == ModuleKind::MK_MainFile;
+
   if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     CD->NumCtorInitializers = Record.readInt();
     if (CD->NumCtorInitializers)
       CD->CtorInitializers = ReadGlobalOffset();
   }
+
   // Store the offset of the body so we can lazily load it later.
-  Reader.PendingBodies[FD] = GetCurrentCursorOffset();
-  HasPendingBody = true;
+  FD->setLazyBody(GetCurrentCursorOffset());
 }
 
 void ASTDeclReader::Visit(Decl *D) {
@@ -913,10 +961,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
 void ASTDeclReader::VisitObjCMethodDecl(ObjCMethodDecl *MD) {
   VisitNamedDecl(MD);
   if (Record.readInt()) {
-    // Load the body on-demand. Most clients won't care, because method
-    // definitions rarely show up in headers.
-    Reader.PendingBodies[MD] = GetCurrentCursorOffset();
-    HasPendingBody = true;
+    // Load the body on-demand (if we don't already have a definition). Most
+    // clients won't care, because method definitions rarely show up in
+    // headers.
+    MD->setLazyBody(GetCurrentCursorOffset());
     MD->setSelfDecl(ReadDeclAs<ImplicitParamDecl>());
     MD->setCmdDecl(ReadDeclAs<ImplicitParamDecl>());
   }
@@ -2567,7 +2615,7 @@ inline void ASTReader::LoadedDecl(unsigned Index, Decl *D) {
 /// This routine should return true for anything that might affect
 /// code generation, e.g., inline function definitions, Objective-C
 /// declarations with metadata, etc.
-static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D, bool HasBody) {
+static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D) {
   // An ObjCMethodDecl is never considered as "interesting" because its
   // implementation container always is.
 
@@ -2593,7 +2641,7 @@ static bool isConsumerInterestedIn(ASTContext &Ctx, Decl *D, bool HasBody) {
     return Var->isFileVarDecl() &&
            Var->isThisDeclarationADefinition() == VarDecl::Definition;
   if (FunctionDecl *Func = dyn_cast<FunctionDecl>(D))
-    return Func->doesThisDeclarationHaveABody() || HasBody;
+    return Func->doesThisDeclarationHaveABody();
 
   if (auto *ES = D->getASTContext().getExternalSource())
     if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never)
@@ -3658,8 +3706,7 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) {
   // AST consumer might need to know about, queue it.
   // We don't pass it to the consumer immediately because we may be in recursive
   // loading, and some declarations may still be initializing.
-  PotentiallyInterestingDecls.push_back(
-      InterestingDecl(D, Reader.hasPendingBody()));
+  PotentiallyInterestingDecls.push_back(D);
 
   return D;
 }
@@ -3682,10 +3729,10 @@ void ASTReader::PassInterestingDeclsToConsumer() {
   EagerlyDeserializedDecls.clear();
 
   while (!PotentiallyInterestingDecls.empty()) {
-    InterestingDecl D = PotentiallyInterestingDecls.front();
+    Decl *D = PotentiallyInterestingDecls.front();
     PotentiallyInterestingDecls.pop_front();
-    if (isConsumerInterestedIn(getContext(), D.getDecl(), D.hasPendingBody()))
-      PassInterestingDeclToConsumer(D.getDecl());
+    if (isConsumerInterestedIn(getContext(), D))
+      PassInterestingDeclToConsumer(D);
   }
 }
 
@@ -3709,7 +3756,7 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
     // to isConsumerInterestedIn because it is unsafe to call in the
     // current ASTReader state.
     bool WasInteresting =
-        Record.JustLoaded || isConsumerInterestedIn(getContext(), D, false);
+        Record.JustLoaded || isConsumerInterestedIn(getContext(), D);
     for (auto &FileAndOffset : UpdateOffsets) {
       ModuleFile *F = FileAndOffset.first;
       uint64_t Offset = FileAndOffset.second;
@@ -3728,10 +3775,8 @@ void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord &Record) {
 
       // We might have made this declaration interesting. If so, remember that
       // we need to hand it off to the consumer.
-      if (!WasInteresting &&
-          isConsumerInterestedIn(getContext(), D, Reader.hasPendingBody())) {
-        PotentiallyInterestingDecls.push_back(
-            InterestingDecl(D, Reader.hasPendingBody()));
+      if (!WasInteresting && isConsumerInterestedIn(getContext(), D)) {
+        PotentiallyInterestingDecls.push_back(D);
         WasInteresting = true;
       }
     }
@@ -4029,12 +4074,6 @@ void ASTDeclReader::UpdateDecl(Decl *D,
 
     case UPD_CXX_ADDED_FUNCTION_DEFINITION: {
       FunctionDecl *FD = cast<FunctionDecl>(D);
-      if (Reader.PendingBodies[FD]) {
-        // FIXME: Maybe check for ODR violations.
-        // It's safe to stop now because this update record is always last.
-        return;
-      }
-
       if (Record.readInt()) {
         // Maintain AST consistency: any later redeclarations of this function
         // are inline if this one is. (We might have merged another declaration