r241999 - [modules] Improve performance when there is a local declaration of an entity

Richard Smith richard-llvm at metafoo.co.uk
Sun Jul 12 16:43:21 PDT 2015


Author: rsmith
Date: Sun Jul 12 18:43:21 2015
New Revision: 241999

URL: http://llvm.org/viewvc/llvm-project?rev=241999&view=rev
Log:
[modules] Improve performance when there is a local declaration of an entity
before the first imported declaration.

We don't need to track all formerly-canonical declarations of an entity; it's sufficient to track those ones for which no other formerly-canonical declaration was imported into the same module. We call those ones "key declarations", and use them as our starting points for collecting redeclarations and performing namespace lookups.

Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/include/clang/Serialization/ASTWriter.h
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/lib/Serialization/ASTWriterDecl.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=241999&r1=241998&r2=241999&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Sun Jul 12 18:43:21 2015
@@ -976,12 +976,14 @@ private:
       MergedLookups;
 
   typedef llvm::DenseMap<Decl *, SmallVector<serialization::DeclID, 2> >
-    MergedDeclsMap;
+    KeyDeclsMap;
     
-  /// \brief A mapping from canonical declarations to the set of additional
-  /// (global, previously-canonical) declaration IDs that have been merged with
-  /// that canonical declaration.
-  MergedDeclsMap MergedDecls;
+  /// \brief A mapping from canonical declarations to the set of global
+  /// declaration IDs for key declaration that have been merged with that
+  /// canonical declaration. A key declaration is a formerly-canonical
+  /// declaration whose module did not import any other key declaration for that
+  /// entity. These are the IDs that we use as keys when finding redecl chains.
+  KeyDeclsMap KeyDecls;
   
   /// \brief A mapping from DeclContexts to the semantic DeclContext that we
   /// are treating as the definition of the entity. This is used, for instance,
@@ -1054,6 +1056,36 @@ public:
   void ResolveImportedPath(ModuleFile &M, std::string &Filename);
   static void ResolveImportedPath(std::string &Filename, StringRef Prefix);
 
+  /// \brief Returns the first key declaration for the given declaration. This
+  /// is one that is formerly-canonical (or still canonical) and whose module
+  /// did not import any other key declaration of the entity.
+  Decl *getKeyDeclaration(Decl *D) {
+    D = D->getCanonicalDecl();
+    if (D->isFromASTFile())
+      return D;
+
+    auto I = KeyDecls.find(D);
+    if (I == KeyDecls.end() || I->second.empty())
+      return D;
+    return GetExistingDecl(I->second[0]);
+  }
+  const Decl *getKeyDeclaration(const Decl *D) {
+    return getKeyDeclaration(const_cast<Decl*>(D));
+  }
+
+  /// \brief Run a callback on each imported key declaration of \p D.
+  template <typename Fn>
+  void forEachImportedKeyDecl(const Decl *D, Fn Visit) {
+    D = D->getCanonicalDecl();
+    if (D->isFromASTFile())
+      Visit(D);
+
+    auto It = KeyDecls.find(const_cast<Decl*>(D));
+    if (It != KeyDecls.end())
+      for (auto ID : It->second)
+        Visit(GetExistingDecl(ID));
+  }
+
 private:
   struct ImportedModule {
     ModuleFile *Mod;
@@ -1124,18 +1156,6 @@ private:
   /// merged into its redecl chain.
   Decl *getMostRecentExistingDecl(Decl *D);
 
-  template <typename Fn>
-  void forEachFormerlyCanonicalImportedDecl(const Decl *D, Fn Visit) {
-    D = D->getCanonicalDecl();
-    if (D->isFromASTFile())
-      Visit(D);
-
-    auto It = MergedDecls.find(const_cast<Decl*>(D));
-    if (It != MergedDecls.end())
-      for (auto ID : It->second)
-        Visit(GetExistingDecl(ID));
-  }
-
   RecordLocation DeclCursorForID(serialization::DeclID ID,
                                  unsigned &RawLocation);
   void loadDeclUpdateRecords(serialization::DeclID ID, Decl *D);

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=241999&r1=241998&r2=241999&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Sun Jul 12 18:43:21 2015
@@ -398,7 +398,7 @@ private:
                  
   /// \brief The set of declarations that may have redeclaration chains that
   /// need to be serialized.
-  llvm::SmallSetVector<Decl *, 4> Redeclarations;
+  llvm::SmallVector<const Decl *, 16> Redeclarations;
                                       
   /// \brief Statements that we've encountered while serializing a
   /// declaration or type.

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=241999&r1=241998&r2=241999&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Sun Jul 12 18:43:21 2015
@@ -6071,7 +6071,7 @@ Decl *ASTReader::GetExistingDecl(DeclID
     if (D) {
       // Track that we have merged the declaration with ID \p ID into the
       // pre-existing predefined declaration \p D.
-      auto &Merged = MergedDecls[D->getCanonicalDecl()];
+      auto &Merged = KeyDecls[D->getCanonicalDecl()];
       if (Merged.empty())
         Merged.push_back(ID);
     }
@@ -6413,10 +6413,10 @@ ASTReader::FindExternalVisibleDeclsByNam
   Contexts.push_back(DC);
 
   if (DC->isNamespace()) {
-    auto Merged = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
-    if (Merged != MergedDecls.end()) {
-      for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)
-        Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
+    auto Key = KeyDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
+    if (Key != KeyDecls.end()) {
+      for (unsigned I = 0, N = Key->second.size(); I != N; ++I)
+        Contexts.push_back(cast<DeclContext>(GetDecl(Key->second[I])));
     }
   }
 
@@ -6533,11 +6533,11 @@ void ASTReader::completeVisibleDeclsMap(
   Contexts.push_back(DC);
 
   if (DC->isNamespace()) {
-    MergedDeclsMap::iterator Merged
-      = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
-    if (Merged != MergedDecls.end()) {
-      for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)
-        Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
+    KeyDeclsMap::iterator Key =
+        KeyDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
+    if (Key != KeyDecls.end()) {
+      for (unsigned I = 0, N = Key->second.size(); I != N; ++I)
+        Contexts.push_back(cast<DeclContext>(GetDecl(Key->second[I])));
     }
   }
 

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=241999&r1=241998&r2=241999&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sun Jul 12 18:43:21 2015
@@ -128,20 +128,22 @@ namespace clang {
       GlobalDeclID FirstID;
       Decl *MergeWith;
       mutable bool Owning;
+      bool IsKeyDecl;
       Decl::Kind DeclKind;
 
       void operator=(RedeclarableResult &) = delete;
 
     public:
       RedeclarableResult(ASTReader &Reader, GlobalDeclID FirstID,
-                         Decl *MergeWith, Decl::Kind DeclKind)
+                         Decl *MergeWith, Decl::Kind DeclKind,
+                         bool IsKeyDecl)
         : Reader(Reader), FirstID(FirstID), MergeWith(MergeWith),
-          Owning(true), DeclKind(DeclKind) {}
+          Owning(true), IsKeyDecl(IsKeyDecl), DeclKind(DeclKind) {}
 
       RedeclarableResult(RedeclarableResult &&Other)
         : Reader(Other.Reader), FirstID(Other.FirstID),
           MergeWith(Other.MergeWith), Owning(Other.Owning),
-          DeclKind(Other.DeclKind) {
+          IsKeyDecl(Other.IsKeyDecl), DeclKind(Other.DeclKind) {
         Other.Owning = false;
       }
 
@@ -156,6 +158,9 @@ namespace clang {
       /// \brief Retrieve the first ID.
       GlobalDeclID getFirstID() const { return FirstID; }
 
+      /// \brief Is this declaration the key declaration?
+      bool isKeyDecl() const { return IsKeyDecl; }
+
       /// \brief Get a known declaration that this should be merged with, if
       /// any.
       Decl *getKnownMergeTarget() const { return MergeWith; }
@@ -348,7 +353,7 @@ namespace clang {
 
     void mergeTemplatePattern(RedeclarableTemplateDecl *D,
                               RedeclarableTemplateDecl *Existing,
-                              DeclID DsID);
+                              DeclID DsID, bool IsKeyDecl);
 
     ObjCTypeParamList *ReadObjCTypeParamList();
 
@@ -2173,12 +2178,16 @@ ASTDeclReader::RedeclarableResult
 ASTDeclReader::VisitRedeclarable(Redeclarable<T> *D) {
   DeclID FirstDeclID = ReadDeclID(Record, Idx);
   Decl *MergeWith = nullptr;
+  bool IsKeyDecl = ThisDeclID == FirstDeclID;
 
   // 0 indicates that this declaration was the only declaration of its entity,
   // and is used for space optimization.
-  if (FirstDeclID == 0)
+  if (FirstDeclID == 0) {
     FirstDeclID = ThisDeclID;
-  else if (unsigned N = Record[Idx++]) {
+    IsKeyDecl = true;
+  } else if (unsigned N = Record[Idx++]) {
+    IsKeyDecl = false;
+
     // We have some declarations that must be before us in our redeclaration
     // chain. Read them now, and remember that we ought to merge with one of
     // them.
@@ -2204,7 +2213,7 @@ ASTDeclReader::VisitRedeclarable(Redecla
   // The result structure takes care to note that we need to load the 
   // other declaration chains for this ID.
   return RedeclarableResult(Reader, FirstDeclID, MergeWith,
-                            static_cast<T *>(D)->getKind());
+                            static_cast<T *>(D)->getKind(), IsKeyDecl);
 }
 
 /// \brief Attempts to merge the given declaration (D) with another declaration
@@ -2243,11 +2252,12 @@ template<typename T> static T assert_cas
 /// declarations.
 void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D,
                                          RedeclarableTemplateDecl *Existing,
-                                         DeclID DsID) {
+                                         DeclID DsID, bool IsKeyDecl) {
   auto *DPattern = D->getTemplatedDecl();
   auto *ExistingPattern = Existing->getTemplatedDecl();
   RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(),
-                            /*MergeWith*/ExistingPattern, DPattern->getKind());
+                            /*MergeWith*/ExistingPattern, DPattern->getKind(),
+                            IsKeyDecl);
 
   if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
     // Merge with any existing definition.
@@ -2310,11 +2320,11 @@ void ASTDeclReader::mergeRedeclarable(Re
     if (auto *DTemplate = dyn_cast<RedeclarableTemplateDecl>(D))
       mergeTemplatePattern(
           DTemplate, assert_cast<RedeclarableTemplateDecl*>(ExistingCanon),
-          TemplatePatternID);
+          TemplatePatternID, Redecl.isKeyDecl());
 
-    // If this declaration was the canonical declaration, make a note of that.
-    if (DCanon == D) {
-      Reader.MergedDecls[ExistingCanon].push_back(Redecl.getFirstID());
+    // If this declaration is a key declaration, make a note of that.
+    if (Redecl.isKeyDecl()) {
+      Reader.KeyDecls[ExistingCanon].push_back(Redecl.getFirstID());
       if (Reader.PendingDeclChainsKnown.insert(ExistingCanon).second)
         Reader.PendingDeclChains.push_back(ExistingCanon);
     }
@@ -3468,7 +3478,7 @@ namespace {
         
         return;
       }
-      
+
       // Dig out all of the redeclarations.
       unsigned Offset = Result->Offset;
       unsigned N = M.RedeclarationChains[Offset];
@@ -3531,9 +3541,9 @@ void ASTReader::loadPendingDeclChain(Dec
   GlobalDeclID CanonID = CanonDecl->getGlobalID();
   if (CanonID)
     SearchDecls.push_back(CanonDecl->getGlobalID()); // Always first.
-  MergedDeclsMap::iterator MergedPos = MergedDecls.find(CanonDecl);
-  if (MergedPos != MergedDecls.end())
-    SearchDecls.append(MergedPos->second.begin(), MergedPos->second.end());
+  KeyDeclsMap::iterator KeyPos = KeyDecls.find(CanonDecl);
+  if (KeyPos != KeyDecls.end())
+    SearchDecls.append(KeyPos->second.begin(), KeyPos->second.end());
 
   // Build up the list of redeclarations.
   RedeclChainVisitor Visitor(*this, SearchDecls, RedeclsDeserialized, CanonID);

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=241999&r1=241998&r2=241999&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Sun Jul 12 18:43:21 2015
@@ -3634,6 +3634,55 @@ ASTWriter::GenerateNameLookupTable(const
 /// bitstream, or 0 if no block was written.
 uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context,
                                                  DeclContext *DC) {
+  // If we imported a key declaration of this namespace, write the visible
+  // lookup results as an update record for it rather than including them
+  // on this declaration. We will only look at key declarations on reload.
+  if (isa<NamespaceDecl>(DC) && Chain &&
+      Chain->getKeyDeclaration(cast<Decl>(DC))->isFromASTFile()) {
+    // Only do this once, for the first local declaration of the namespace.
+    for (NamespaceDecl *Prev = cast<NamespaceDecl>(DC)->getPreviousDecl(); Prev;
+         Prev = Prev->getPreviousDecl())
+      if (!Prev->isFromASTFile())
+        return 0;
+
+    // Note that we need to emit an update record for the primary context.
+    UpdatedDeclContexts.insert(DC->getPrimaryContext());
+
+    // Make sure all visible decls are written. They will be recorded later. We
+    // do this using a side data structure so we can sort the names into
+    // a deterministic order.
+    StoredDeclsMap *Map = DC->getPrimaryContext()->buildLookup();
+    SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>, 16>
+        LookupResults;
+    if (Map) {
+      LookupResults.reserve(Map->size());
+      for (auto &Entry : *Map)
+        LookupResults.push_back(
+            std::make_pair(Entry.first, Entry.second.getLookupResult()));
+    }
+
+    std::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first());
+    for (auto &NameAndResult : LookupResults) {
+      DeclarationName Name = NameAndResult.first;
+      DeclContext::lookup_result Result = NameAndResult.second;
+      if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
+          Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {
+        // We have to work around a name lookup bug here where negative lookup
+        // results for these names get cached in namespace lookup tables (these
+        // names should never be looked up in a namespace).
+        assert(Result.empty() && "Cannot have a constructor or conversion "
+                                 "function name in a namespace!");
+        continue;
+      }
+
+      for (NamedDecl *ND : Result)
+        if (!ND->isFromASTFile())
+          GetDeclRef(ND);
+    }
+
+    return 0;
+  }
+
   if (DC->getPrimaryContext() != DC)
     return 0;
 
@@ -3685,6 +3734,11 @@ void ASTWriter::WriteDeclContextVisibleU
   SmallString<4096> LookupTable;
   uint32_t BucketOffset = GenerateNameLookupTable(DC, LookupTable);
 
+  // If we're updating a namespace, select a key declaration as the key for the
+  // update record; those are the only ones that will be checked on reload.
+  if (isa<NamespaceDecl>(DC))
+    DC = cast<DeclContext>(Chain->getKeyDeclaration(cast<Decl>(DC)));
+
   // Write the lookup table
   RecordData Record;
   Record.push_back(UPDATE_VISIBLE);
@@ -3717,11 +3771,17 @@ void ASTWriter::WriteRedeclarations() {
   SmallVector<serialization::LocalRedeclarationsInfo, 2> LocalRedeclsMap;
 
   for (unsigned I = 0, N = Redeclarations.size(); I != N; ++I) {
-    Decl *First = Redeclarations[I];
-    assert(First->isFirstDecl() && "Not the first declaration?");
-    
-    Decl *MostRecent = First->getMostRecentDecl();
-    
+    const Decl *Key = Redeclarations[I];
+    assert((Chain ? Chain->getKeyDeclaration(Key) == Key
+                  : Key->isFirstDecl()) &&
+           "not the key declaration");
+
+    const Decl *First = Key->getCanonicalDecl();
+    const Decl *MostRecent = First->getMostRecentDecl();
+
+    assert((getDeclID(First) >= NUM_PREDEF_DECL_IDS || First == Key) &&
+           "should not have imported key decls for predefined decl");
+
     // If we only have a single declaration, there is no point in storing
     // a redeclaration chain.
     if (First == MostRecent)
@@ -3730,34 +3790,34 @@ void ASTWriter::WriteRedeclarations() {
     unsigned Offset = LocalRedeclChains.size();
     unsigned Size = 0;
     LocalRedeclChains.push_back(0); // Placeholder for the size.
-    
+
     // Collect the set of local redeclarations of this declaration.
-    for (Decl *Prev = MostRecent; Prev != First;
+    for (const Decl *Prev = MostRecent; Prev;
          Prev = Prev->getPreviousDecl()) { 
-      if (!Prev->isFromASTFile()) {
+      if (!Prev->isFromASTFile() && Prev != Key) {
         AddDeclRef(Prev, LocalRedeclChains);
         ++Size;
       }
     }
 
     LocalRedeclChains[Offset] = Size;
-    
+
     // Reverse the set of local redeclarations, so that we store them in
     // order (since we found them in reverse order).
     std::reverse(LocalRedeclChains.end() - Size, LocalRedeclChains.end());
-    
+
     // Add the mapping from the first ID from the AST to the set of local
     // declarations.
-    LocalRedeclarationsInfo Info = { getDeclID(First), Offset };
+    LocalRedeclarationsInfo Info = { getDeclID(Key), Offset };
     LocalRedeclsMap.push_back(Info);
-    
+
     assert(N == Redeclarations.size() && 
            "Deserialized a declaration we shouldn't have");
   }
-  
+
   if (LocalRedeclChains.empty())
     return;
-  
+
   // Sort the local redeclarations map by the first declaration ID,
   // since the reader will be performing binary searches on this information.
   llvm::array_pod_sort(LocalRedeclsMap.begin(), LocalRedeclsMap.end());
@@ -4053,25 +4113,27 @@ void ASTWriter::WriteASTCore(Sema &SemaR
   Preprocessor &PP = SemaRef.PP;
 
   // Set up predefined declaration IDs.
-  DeclIDs[Context.getTranslationUnitDecl()] = PREDEF_DECL_TRANSLATION_UNIT_ID;
-  if (Context.ObjCIdDecl)
-    DeclIDs[Context.ObjCIdDecl] = PREDEF_DECL_OBJC_ID_ID;
-  if (Context.ObjCSelDecl)
-    DeclIDs[Context.ObjCSelDecl] = PREDEF_DECL_OBJC_SEL_ID;
-  if (Context.ObjCClassDecl)
-    DeclIDs[Context.ObjCClassDecl] = PREDEF_DECL_OBJC_CLASS_ID;
-  if (Context.ObjCProtocolClassDecl)
-    DeclIDs[Context.ObjCProtocolClassDecl] = PREDEF_DECL_OBJC_PROTOCOL_ID;
-  if (Context.Int128Decl)
-    DeclIDs[Context.Int128Decl] = PREDEF_DECL_INT_128_ID;
-  if (Context.UInt128Decl)
-    DeclIDs[Context.UInt128Decl] = PREDEF_DECL_UNSIGNED_INT_128_ID;
-  if (Context.ObjCInstanceTypeDecl)
-    DeclIDs[Context.ObjCInstanceTypeDecl] = PREDEF_DECL_OBJC_INSTANCETYPE_ID;
-  if (Context.BuiltinVaListDecl)
-    DeclIDs[Context.getBuiltinVaListDecl()] = PREDEF_DECL_BUILTIN_VA_LIST_ID;
-  if (Context.ExternCContext)
-    DeclIDs[Context.ExternCContext] = PREDEF_DECL_EXTERN_C_CONTEXT_ID;
+  auto RegisterPredefDecl = [&] (Decl *D, PredefinedDeclIDs ID) {
+    if (D) {
+      assert(D->isCanonicalDecl() && "predefined decl is not canonical");
+      DeclIDs[D] = ID;
+      if (D->getMostRecentDecl() != D)
+        Redeclarations.push_back(D);
+    }
+  };
+  RegisterPredefDecl(Context.getTranslationUnitDecl(),
+                     PREDEF_DECL_TRANSLATION_UNIT_ID);
+  RegisterPredefDecl(Context.ObjCIdDecl, PREDEF_DECL_OBJC_ID_ID);
+  RegisterPredefDecl(Context.ObjCSelDecl, PREDEF_DECL_OBJC_SEL_ID);
+  RegisterPredefDecl(Context.ObjCClassDecl, PREDEF_DECL_OBJC_CLASS_ID);
+  RegisterPredefDecl(Context.ObjCProtocolClassDecl,
+                     PREDEF_DECL_OBJC_PROTOCOL_ID);
+  RegisterPredefDecl(Context.Int128Decl, PREDEF_DECL_INT_128_ID);
+  RegisterPredefDecl(Context.UInt128Decl, PREDEF_DECL_UNSIGNED_INT_128_ID);
+  RegisterPredefDecl(Context.ObjCInstanceTypeDecl,
+                     PREDEF_DECL_OBJC_INSTANCETYPE_ID);
+  RegisterPredefDecl(Context.BuiltinVaListDecl, PREDEF_DECL_BUILTIN_VA_LIST_ID);
+  RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID);
 
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
@@ -5675,7 +5737,7 @@ void ASTWriter::AddedCXXTemplateSpeciali
 void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {
   assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");
   if (!Chain) return;
-  Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {
+  Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {
     // If we don't already know the exception specification for this redecl
     // chain, add an update record for it.
     if (isUnresolvedExceptionSpec(cast<FunctionDecl>(D)
@@ -5689,7 +5751,7 @@ void ASTWriter::ResolvedExceptionSpec(co
 void ASTWriter::DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) {
   assert(!WritingAST && "Already writing the AST!");
   if (!Chain) return;
-  Chain->forEachFormerlyCanonicalImportedDecl(FD, [&](const Decl *D) {
+  Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {
     DeclUpdates[D].push_back(
         DeclUpdate(UPD_CXX_DEDUCED_RETURN_TYPE, ReturnType));
   });
@@ -5700,7 +5762,7 @@ void ASTWriter::ResolvedOperatorDelete(c
   assert(!WritingAST && "Already writing the AST!");
   assert(Delete && "Not given an operator delete");
   if (!Chain) return;
-  Chain->forEachFormerlyCanonicalImportedDecl(DD, [&](const Decl *D) {
+  Chain->forEachImportedKeyDecl(DD, [&](const Decl *D) {
     DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_RESOLVED_DTOR_DELETE, Delete));
   });
 }

Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=241999&r1=241998&r2=241999&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Sun Jul 12 18:43:21 2015
@@ -254,6 +254,8 @@ void ASTDeclWriter::VisitDecl(Decl *D) {
   //
   // This happens when we instantiate a class with a friend declaration or a
   // function with a local extern declaration, for instance.
+  //
+  // FIXME: Can we handle this in AddedVisibleDecl instead?
   if (D->isOutOfLine()) {
     auto *DC = D->getDeclContext();
     while (auto *NS = dyn_cast<NamespaceDecl>(DC->getRedeclContext())) {
@@ -1005,42 +1007,6 @@ void ASTDeclWriter::VisitNamespaceDecl(N
     Writer.AddDeclRef(D->getAnonymousNamespace(), Record);
   Code = serialization::DECL_NAMESPACE;
 
-  if (Writer.hasChain() && !D->isOriginalNamespace() &&
-      D->getOriginalNamespace()->isFromASTFile()) {
-    NamespaceDecl *NS = D->getOriginalNamespace();
-    Writer.UpdatedDeclContexts.insert(NS);
-
-    // Make sure all visible decls are written. They will be recorded later. We
-    // do this using a side data structure so we can sort the names into
-    // a deterministic order.
-    StoredDeclsMap *Map = NS->buildLookup();
-    SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>, 16>
-        LookupResults;
-    if (Map) {
-      LookupResults.reserve(Map->size());
-      for (auto &Entry : *Map)
-        LookupResults.push_back(
-            std::make_pair(Entry.first, Entry.second.getLookupResult()));
-    }
-
-    std::sort(LookupResults.begin(), LookupResults.end(), llvm::less_first());
-    for (auto &NameAndResult : LookupResults) {
-      DeclarationName Name = NameAndResult.first;
-      DeclContext::lookup_result Result = NameAndResult.second;
-      if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
-          Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {
-        // We have to work around a name lookup bug here where negative lookup
-        // results for these names get cached in namespace lookup tables.
-        assert(Result.empty() && "Cannot have a constructor or conversion "
-                                 "function name in a namespace!");
-        continue;
-      }
-
-      for (NamedDecl *ND : Result)
-        Writer.GetDeclRef(ND);
-    }
-  }
-
   if (Writer.hasChain() && D->isAnonymousNamespace() && 
       D == D->getMostRecentDecl()) {
     // This is a most recent reopening of the anonymous namespace. If its parent
@@ -1512,6 +1478,17 @@ void ASTDeclWriter::VisitDeclContext(Dec
   Record.push_back(VisibleOffset);
 }
 
+/// Determine whether D is the first declaration in its redeclaration chain that
+/// is not from an AST file.
+template <typename T>
+static bool isFirstLocalDecl(Redeclarable<T> *D) {
+  assert(D && !static_cast<T*>(D)->isFromASTFile());
+  do
+    D = D->getPreviousDecl();
+  while (D && static_cast<T*>(D)->isFromASTFile());
+  return !D;
+}
+
 template <typename T>
 void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
   T *First = D->getFirstDecl();
@@ -1520,41 +1497,30 @@ void ASTDeclWriter::VisitRedeclarable(Re
     assert(isRedeclarableDeclKind(static_cast<T *>(D)->getKind()) &&
            "Not considered redeclarable?");
 
-    // There is more than one declaration of this entity, so we will need to
-    // write a redeclaration chain.
     Writer.AddDeclRef(First, Record);
-    Writer.Redeclarations.insert(First);
 
-    auto *Previous = D->getPreviousDecl();
-
-    // In a modules build, we can have imported declarations after a local
-    // canonical declaration. If this is the first local declaration, emit
-    // a list of all such imported declarations so that we can ensure they
-    // are loaded before we are. This allows us to rebuild the redecl chain
-    // in the right order on reload (all declarations imported by a module
-    // should be before all declarations provided by that module).
-    bool EmitImportedMergedCanonicalDecls = false;
+    // In a modules build, emit a list of all imported key declarations
+    // (excluding First, if it was imported), so that we can be sure that all
+    // redeclarations visible to this module are before D in the redecl chain.
+    unsigned I = Record.size();
+    Record.push_back(0);
     if (Context.getLangOpts().Modules && Writer.Chain) {
-      auto *PreviousLocal = Previous;
-      while (PreviousLocal && PreviousLocal->isFromASTFile())
-        PreviousLocal = PreviousLocal->getPreviousDecl();
-      if (!PreviousLocal)
-        EmitImportedMergedCanonicalDecls = true;
+      if (isFirstLocalDecl(D)) {
+        Writer.Chain->forEachImportedKeyDecl(First, [&](const Decl *D) {
+          if (D != First)
+            Writer.AddDeclRef(D, Record);
+        });
+        Record[I] = Record.size() - I - 1;
+
+        // Write a redeclaration chain, attached to the first key decl.
+        Writer.Redeclarations.push_back(Writer.Chain->getKeyDeclaration(First));
+      }
+    } else if (D == First || D->getPreviousDecl()->isFromASTFile()) {
+      assert(isFirstLocalDecl(D) && "imported decl after local decl");
+
+      // Write a redeclaration chain attached to the first decl.
+      Writer.Redeclarations.push_back(First);
     }
-    if (EmitImportedMergedCanonicalDecls) {
-      llvm::SmallMapVector<ModuleFile*, Decl*, 16> FirstInModule;
-      for (auto *Redecl = MostRecent; Redecl;
-           Redecl = Redecl->getPreviousDecl())
-        if (Redecl->isFromASTFile())
-          FirstInModule[Writer.Chain->getOwningModuleFile(Redecl)] = Redecl;
-      // FIXME: If FirstInModule has entries for modules A and B, and B imports
-      // A (directly or indirectly), we don't need to write the entry for A.
-      Record.push_back(FirstInModule.size());
-      for (auto I = FirstInModule.rbegin(), E = FirstInModule.rend();
-           I != E; ++I)
-        Writer.AddDeclRef(I->second, Record);
-    } else
-      Record.push_back(0);
 
     // Make sure that we serialize both the previous and the most-recent 
     // declarations, which (transitively) ensures that all declarations in the
@@ -1562,7 +1528,7 @@ void ASTDeclWriter::VisitRedeclarable(Re
     //
     // FIXME: This is not correct; when we reach an imported declaration we
     // won't emit its previous declaration.
-    (void)Writer.GetDeclRef(Previous);
+    (void)Writer.GetDeclRef(D->getPreviousDecl());
     (void)Writer.GetDeclRef(MostRecent);
   } else {
     // We use the sentinel value 0 to indicate an only declaration.





More information about the cfe-commits mailing list