r233251 - [Modules] Delete a bunch of complex code for ensuring visible decls in

Chandler Carruth chandlerc at gmail.com
Wed Mar 25 21:27:10 PDT 2015


Author: chandlerc
Date: Wed Mar 25 23:27:10 2015
New Revision: 233251

URL: http://llvm.org/viewvc/llvm-project?rev=233251&view=rev
Log:
[Modules] Delete a bunch of complex code for ensuring visible decls in
updated decl contexts get emitted.

Since this code was added, we have newer vastly simpler code for
handling this. The code I'm removing was very expensive and also
generated unstable order of declarations which made module outputs
non-deterministic.

All of the tests continue to pass for me and I'm able to check the
difference between the .pcm files after merging modules together.

Modified:
    cfe/trunk/include/clang/Serialization/ASTWriter.h
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
    cfe/trunk/test/Modules/stress1.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=233251&r1=233250&r2=233251&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Wed Mar 25 23:27:10 2015
@@ -509,9 +509,6 @@ private:
   bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext *DC);
 
-  template<typename Visitor>
-  void visitLocalLookupResults(const DeclContext *DC, Visitor AddLookupResult);
-
   uint32_t GenerateNameLookupTable(const DeclContext *DC,
                                    llvm::SmallVectorImpl<char> &LookupTable);
   uint64_t WriteDeclContextLexicalBlock(ASTContext &Context, DeclContext *DC);
@@ -737,9 +734,6 @@ public:
   /// \brief Add a version tuple to the given record
   void AddVersionTuple(const VersionTuple &Version, RecordDataImpl &Record);
 
-  /// \brief Mark a declaration context as needing an update.
-  void AddUpdatedDeclContext(const DeclContext *DC);
-
   void RewriteDecl(const Decl *D) {
     DeclsToRewrite.insert(D);
   }

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=233251&r1=233250&r2=233251&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Mar 25 23:27:10 2015
@@ -3710,57 +3710,6 @@ bool ASTWriter::isLookupResultEntirelyEx
   return true;
 }
 
-template<typename Visitor>
-void ASTWriter::visitLocalLookupResults(const DeclContext *ConstDC,
-                                        Visitor AddLookupResult) {
-  // FIXME: We need to build the lookups table, which is logically const.
-  DeclContext *DC = const_cast<DeclContext*>(ConstDC);
-  assert(DC == DC->getPrimaryContext() && "only primary DC has lookup table");
-
-  SmallVector<DeclarationName, 16> ExternalNames;
-  for (auto &Lookup : *DC->buildLookup()) {
-    if (isLookupResultExternal(Lookup.second, DC)) {
-      // If there are no local declarations in our lookup result, we don't
-      // need to write an entry for the name at all unless we're rewriting
-      // the decl context. If we can't write out a lookup set without
-      // performing more deserialization, just skip this entry.
-      if (!isRewritten(cast<Decl>(DC)) &&
-          isLookupResultEntirelyExternal(Lookup.second, DC))
-        continue;
-
-      // We don't know for sure what declarations are found by this name,
-      // because the external source might have a different set from the set
-      // that are in the lookup map, and we can't update it now without
-      // risking invalidating our lookup iterator. So add it to a queue to
-      // deal with later.
-      ExternalNames.push_back(Lookup.first);
-      continue;
-    }
-
-    AddLookupResult(Lookup.first, Lookup.second.getLookupResult());
-  }
-
-  // Add the names we needed to defer. Note, this shouldn't add any new decls
-  // to the list we need to serialize: any new declarations we find here should
-  // be imported from an external source.
-  // FIXME: What if the external source isn't an ASTReader?
-  for (const auto &Name : ExternalNames)
-    AddLookupResult(Name, DC->lookup(Name));
-}
-
-void ASTWriter::AddUpdatedDeclContext(const DeclContext *DC) {
-  if (UpdatedDeclContexts.insert(DC).second && WritingAST) {
-    // Ensure we emit all the visible declarations.
-    // FIXME: This code is almost certainly wrong. It is at least failing to
-    // visit all the decls it should.
-    visitLocalLookupResults(DC, [&](DeclarationName Name,
-                                    DeclContext::lookup_result Result) {
-      for (auto *Decl : Result)
-        GetDeclRef(getDeclForLocalLookup(getLangOpts(), Decl));
-    });
-  }
-}
-
 uint32_t
 ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC,
                                    llvm::SmallVectorImpl<char> &LookupTable) {
@@ -4388,15 +4337,6 @@ void ASTWriter::WriteASTCore(Sema &SemaR
       getIdentifierRef(&Table.get(BuiltinNames[I]));
   }
 
-  // If we saw any DeclContext updates before we started writing the AST file,
-  // make sure all visible decls in those DeclContexts are written out.
-  if (!UpdatedDeclContexts.empty()) {
-    auto OldUpdatedDeclContexts = std::move(UpdatedDeclContexts);
-    UpdatedDeclContexts.clear();
-    for (auto *DC : OldUpdatedDeclContexts)
-      AddUpdatedDeclContext(DC);
-  }
-
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
   // headers.
@@ -4861,7 +4801,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(R
 
       case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
         auto *RD = cast<CXXRecordDecl>(D);
-        AddUpdatedDeclContext(RD->getPrimaryContext());
+        UpdatedDeclContexts.insert(RD->getPrimaryContext());
         AddCXXDefinitionData(RD, Record);
         Record.push_back(WriteDeclContextLexicalBlock(
             *Context, const_cast<CXXRecordDecl *>(RD)));
@@ -5923,7 +5863,7 @@ void ASTWriter::AddedVisibleDecl(const D
 
   assert(!getDefinitiveDeclContext(DC) && "DeclContext not definitive!");
   assert(!WritingAST && "Already writing the AST!");
-  AddUpdatedDeclContext(DC);
+  UpdatedDeclContexts.insert(DC);
   UpdatingVisibleDecls.push_back(D);
 }
 

Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=233251&r1=233250&r2=233251&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Mar 25 23:27:10 2015
@@ -243,7 +243,7 @@ void ASTDeclWriter::VisitDecl(Decl *D) {
     while (auto *NS = dyn_cast<NamespaceDecl>(DC->getRedeclContext())) {
       if (!NS->isFromASTFile())
         break;
-      Writer.AddUpdatedDeclContext(NS->getPrimaryContext());
+      Writer.UpdatedDeclContexts.insert(NS->getPrimaryContext());
       if (!NS->isInlineNamespace())
         break;
       DC = NS->getParent();
@@ -978,7 +978,7 @@ void ASTDeclWriter::VisitNamespaceDecl(N
   if (Writer.hasChain() && !D->isOriginalNamespace() &&
       D->getOriginalNamespace()->isFromASTFile()) {
     NamespaceDecl *NS = D->getOriginalNamespace();
-    Writer.AddUpdatedDeclContext(NS);
+    Writer.UpdatedDeclContexts.insert(NS);
 
     // Make sure all visible decls are written. They will be recorded later.
     if (StoredDeclsMap *Map = NS->buildLookup()) {

Modified: cfe/trunk/test/Modules/stress1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/stress1.cpp?rev=233251&r1=233250&r2=233251&view=diff
==============================================================================
--- cfe/trunk/test/Modules/stress1.cpp (original)
+++ cfe/trunk/test/Modules/stress1.cpp Wed Mar 25 23:27:10 2015
@@ -60,7 +60,7 @@
 // RUN:   -emit-module -fmodule-name=merge00 -o %t/merge00_check.pcm \
 // RUN:   Inputs/stress1/module.modulemap
 //
-// FIXME: diff %t/merge00.pcm %t/merge00_check.pcm
+// RUN: diff %t/merge00.pcm %t/merge00_check.pcm
 //
 // RUN: %clang_cc1 -fmodules -x c++ -std=c++11 \
 // RUN:   -I Inputs/stress1 \





More information about the cfe-commits mailing list