r276473 - [modules] Teach the ASTWriter to ignore mutations coming from the ASTReader.

Vassil Vassilev via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 22 14:08:24 PDT 2016


Author: vvassilev
Date: Fri Jul 22 16:08:24 2016
New Revision: 276473

URL: http://llvm.org/viewvc/llvm-project?rev=276473&view=rev
Log:
[modules] Teach the ASTWriter to ignore mutations coming from the ASTReader.

Processing update records (and loading a module, in general) might trigger
unexpected calls to the ASTWriter (being a mutation listener). Now we have a
mechanism to suppress those calls to the ASTWriter but notify other possible
mutation listeners.

Fixes https://llvm.org/bugs/show_bug.cgi?id=28332

Patch by Cristina Cristescu and me.

Reviewed by Richard Smith (D21800).

Added:
    cfe/trunk/test/Modules/Inputs/PR28332/
    cfe/trunk/test/Modules/Inputs/PR28332/TextualInclude.h
    cfe/trunk/test/Modules/Inputs/PR28332/a.h
    cfe/trunk/test/Modules/Inputs/PR28332/b.h
    cfe/trunk/test/Modules/Inputs/PR28332/c.h
    cfe/trunk/test/Modules/Inputs/PR28332/module.modulemap
    cfe/trunk/test/Modules/pr28332.cpp
Modified:
    cfe/trunk/include/clang/Serialization/ASTReader.h
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=276473&r1=276472&r2=276473&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jul 22 16:08:24 2016
@@ -842,6 +842,9 @@ private:
   /// \brief Whether we have tried loading the global module index yet.
   bool TriedLoadingGlobalIndex;
 
+  ///\brief Whether we are currently processing update records.
+  bool ProcessingUpdateRecords;
+
   typedef llvm::DenseMap<unsigned, SwitchCase *> SwitchCaseMapTy;
   /// \brief Mapping from switch-case IDs in the chain to switch-case statements
   ///
@@ -1041,6 +1044,23 @@ private:
     ~ReadingKindTracker() { Reader.ReadingKind = PrevKind; }
   };
 
+  /// \brief RAII object to mark the start of processing updates.
+  class ProcessingUpdatesRAIIObj {
+    ASTReader &Reader;
+    bool PrevState;
+
+    ProcessingUpdatesRAIIObj(const ProcessingUpdatesRAIIObj &) = delete;
+    void operator=(const ProcessingUpdatesRAIIObj &) = delete;
+
+  public:
+    ProcessingUpdatesRAIIObj(ASTReader &reader)
+      : Reader(reader), PrevState(Reader.ProcessingUpdateRecords) {
+      Reader.ProcessingUpdateRecords = true;
+    }
+
+    ~ProcessingUpdatesRAIIObj() { Reader.ProcessingUpdateRecords = PrevState; }
+  };
+
   /// \brief Suggested contents of the predefines buffer, after this
   /// PCH file has been processed.
   ///
@@ -2129,6 +2149,8 @@ public:
 
   /// \brief Loads comments ranges.
   void ReadComments() override;
+
+  bool isProcessingUpdateRecords() { return ProcessingUpdateRecords; }
 };
 
 /// \brief Helper class that saves the current stream position and

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=276473&r1=276472&r2=276473&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Jul 22 16:08:24 2016
@@ -8644,6 +8644,7 @@ void ASTReader::FinishedDeserializing()
       auto Updates = std::move(PendingExceptionSpecUpdates);
       PendingExceptionSpecUpdates.clear();
       for (auto Update : Updates) {
+        ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
         auto *FPT = Update.second->getType()->castAs<FunctionProtoType>();
         auto ESI = FPT->getExtProtoInfo().ExceptionSpec;
         if (auto *Listener = Context.getASTMutationListener())
@@ -8714,6 +8715,7 @@ ASTReader::ASTReader(
       AllowConfigurationMismatch(AllowConfigurationMismatch),
       ValidateSystemInputs(ValidateSystemInputs),
       UseGlobalIndex(UseGlobalIndex), TriedLoadingGlobalIndex(false),
+      ProcessingUpdateRecords(false),
       CurrSwitchCaseStmts(&SwitchCaseStmts), NumSLocEntriesRead(0),
       TotalNumSLocEntries(0), NumStatementsRead(0), TotalNumStatements(0),
       NumMacrosRead(0), TotalNumMacros(0), NumIdentifierLookups(0),

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=276473&r1=276472&r2=276473&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jul 22 16:08:24 2016
@@ -3489,6 +3489,7 @@ void ASTReader::loadDeclUpdateRecords(se
   // The declaration may have been modified by files later in the chain.
   // If this is the case, read the record containing the updates from each file
   // and pass it to ASTDeclReader to make the modifications.
+  ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
   DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
   if (UpdI != DeclUpdateOffsets.end()) {
     auto UpdateOffsets = std::move(UpdI->second);
@@ -3907,11 +3908,8 @@ void ASTDeclReader::UpdateDecl(Decl *D,
     }
 
     case UPD_DECL_MARKED_USED: {
-      // FIXME: This doesn't send the right notifications if there are
-      // ASTMutationListeners other than an ASTWriter.
-
       // Maintain AST consistency: any later redeclarations are used too.
-      D->setIsUsed();
+      D->markUsed(Reader.Context);
       break;
     }
 
@@ -3935,11 +3933,8 @@ void ASTDeclReader::UpdateDecl(Decl *D,
         Exported = TD->getDefinition();
       Module *Owner = SubmoduleID ? Reader.getSubmodule(SubmoduleID) : nullptr;
       if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
-        // FIXME: This doesn't send the right notifications if there are
-        // ASTMutationListeners other than an ASTWriter.
-        Reader.getContext().mergeDefinitionIntoModule(
-            cast<NamedDecl>(Exported), Owner,
-            /*NotifyListeners*/ false);
+        Reader.getContext().mergeDefinitionIntoModule(cast<NamedDecl>(Exported),
+                                                      Owner);
         Reader.PendingMergedDefinitionsToDeduplicate.insert(
             cast<NamedDecl>(Exported));
       } else if (Owner && Owner->NameVisibility != Module::AllVisible) {

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=276473&r1=276472&r2=276473&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Jul 22 16:08:24 2016
@@ -5657,6 +5657,7 @@ void ASTWriter::ModuleRead(serialization
 }
 
 void ASTWriter::CompletedTagDefinition(const TagDecl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(D->isCompleteDefinition());
   assert(!WritingAST && "Already writing the AST!");
   if (auto *RD = dyn_cast<CXXRecordDecl>(D)) {
@@ -5683,7 +5684,8 @@ static bool isImportedDeclContext(ASTRea
 }
 
 void ASTWriter::AddedVisibleDecl(const DeclContext *DC, const Decl *D) {
-   assert(DC->isLookupContext() &&
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
+  assert(DC->isLookupContext() &&
           "Should not add lookup results to non-lookup contexts!");
 
   // TU is handled elsewhere.
@@ -5717,6 +5719,7 @@ void ASTWriter::AddedVisibleDecl(const D
 }
 
 void ASTWriter::AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(D->isImplicit());
 
   // We're only interested in cases where a local declaration is added to an
@@ -5734,6 +5737,7 @@ void ASTWriter::AddedCXXImplicitMember(c
 }
 
 void ASTWriter::ResolvedExceptionSpec(const FunctionDecl *FD) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!DoneWritingDeclsAndTypes && "Already done writing updates!");
   if (!Chain) return;
   Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {
@@ -5748,6 +5752,7 @@ void ASTWriter::ResolvedExceptionSpec(co
 }
 
 void ASTWriter::DeducedReturnType(const FunctionDecl *FD, QualType ReturnType) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!Chain) return;
   Chain->forEachImportedKeyDecl(FD, [&](const Decl *D) {
@@ -5758,6 +5763,7 @@ void ASTWriter::DeducedReturnType(const
 
 void ASTWriter::ResolvedOperatorDelete(const CXXDestructorDecl *DD,
                                        const FunctionDecl *Delete) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   assert(Delete && "Not given an operator delete");
   if (!Chain) return;
@@ -5767,6 +5773,7 @@ void ASTWriter::ResolvedOperatorDelete(c
 }
 
 void ASTWriter::CompletedImplicitDefinition(const FunctionDecl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!D->isFromASTFile())
     return; // Declaration not imported from PCH.
@@ -5776,6 +5783,7 @@ void ASTWriter::CompletedImplicitDefinit
 }
 
 void ASTWriter::FunctionDefinitionInstantiated(const FunctionDecl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!D->isFromASTFile())
     return;
@@ -5784,6 +5792,7 @@ void ASTWriter::FunctionDefinitionInstan
 }
 
 void ASTWriter::StaticDataMemberInstantiated(const VarDecl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!D->isFromASTFile())
     return;
@@ -5796,6 +5805,7 @@ void ASTWriter::StaticDataMemberInstanti
 }
 
 void ASTWriter::DefaultArgumentInstantiated(const ParmVarDecl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!D->isFromASTFile())
     return;
@@ -5806,6 +5816,7 @@ void ASTWriter::DefaultArgumentInstantia
 
 void ASTWriter::AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
                                              const ObjCInterfaceDecl *IFD) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!IFD->isFromASTFile())
     return; // Declaration not imported from PCH.
@@ -5816,6 +5827,7 @@ void ASTWriter::AddedObjCCategoryToInter
 }
 
 void ASTWriter::DeclarationMarkedUsed(const Decl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
 
   // If there is *any* declaration of the entity that's not from an AST file,
@@ -5829,6 +5841,7 @@ void ASTWriter::DeclarationMarkedUsed(co
 }
 
 void ASTWriter::DeclarationMarkedOpenMPThreadPrivate(const Decl *D) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!D->isFromASTFile())
     return;
@@ -5838,6 +5851,7 @@ void ASTWriter::DeclarationMarkedOpenMPT
 
 void ASTWriter::DeclarationMarkedOpenMPDeclareTarget(const Decl *D,
                                                      const Attr *Attr) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!D->isFromASTFile())
     return;
@@ -5847,6 +5861,7 @@ void ASTWriter::DeclarationMarkedOpenMPD
 }
 
 void ASTWriter::RedefinedHiddenDefinition(const NamedDecl *D, Module *M) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   assert(D->isHidden() && "expected a hidden declaration");
   DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_EXPORTED, M));
@@ -5854,6 +5869,7 @@ void ASTWriter::RedefinedHiddenDefinitio
 
 void ASTWriter::AddedAttributeToRecord(const Attr *Attr,
                                        const RecordDecl *Record) {
+  if (Chain && Chain->isProcessingUpdateRecords()) return;
   assert(!WritingAST && "Already writing the AST!");
   if (!Record->isFromASTFile())
     return;

Added: cfe/trunk/test/Modules/Inputs/PR28332/TextualInclude.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR28332/TextualInclude.h?rev=276473&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR28332/TextualInclude.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR28332/TextualInclude.h Fri Jul 22 16:08:24 2016
@@ -0,0 +1,7 @@
+#ifndef LLVM_ADT_SMALLVECTORIMPL_H
+#define LLVM_ADT_SMALLVECTORIMPL_H
+class SmallVectorImpl {
+public:
+  ~SmallVectorImpl();
+};
+#endif
\ No newline at end of file

Added: cfe/trunk/test/Modules/Inputs/PR28332/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR28332/a.h?rev=276473&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR28332/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR28332/a.h Fri Jul 22 16:08:24 2016
@@ -0,0 +1,8 @@
+#include "b.h"
+
+class A {
+  SmallVector<char, 8> LegalIntWidths;
+  A() {}
+};
+
+#include "c.h"

Added: cfe/trunk/test/Modules/Inputs/PR28332/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR28332/b.h?rev=276473&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR28332/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR28332/b.h Fri Jul 22 16:08:24 2016
@@ -0,0 +1,3 @@
+#include "TextualInclude.h"
+template <typename, int> class SmallVector : SmallVectorImpl {};
+

Added: cfe/trunk/test/Modules/Inputs/PR28332/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR28332/c.h?rev=276473&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR28332/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/PR28332/c.h Fri Jul 22 16:08:24 2016
@@ -0,0 +1,2 @@
+#include "TextualInclude.h"
+

Added: cfe/trunk/test/Modules/Inputs/PR28332/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR28332/module.modulemap?rev=276473&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/PR28332/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/PR28332/module.modulemap Fri Jul 22 16:08:24 2016
@@ -0,0 +1,3 @@
+module "c.h" { header "c.h" export * }
+module "b.h" { header "b.h" export * }
+module "a.h" { header "a.h" }

Added: cfe/trunk/test/Modules/pr28332.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr28332.cpp?rev=276473&view=auto
==============================================================================
--- cfe/trunk/test/Modules/pr28332.cpp (added)
+++ cfe/trunk/test/Modules/pr28332.cpp Fri Jul 22 16:08:24 2016
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR28332 -verify %s
+// RUN: %clang_cc1 -std=c++11 -fmodules -fmodule-map-file=%S/Inputs/PR28332/module.modulemap -fmodules-cache-path=%t -I%S/Inputs/PR28332 -verify %s
+
+#include "a.h"
+
+// expected-no-diagnostics
+




More information about the cfe-commits mailing list