r235420 - [modules] Move list of exported module macros from IdentifierInfo lookup table to separate storage, adjacent to the macro directive history.

Richard Smith richard-llvm at metafoo.co.uk
Tue Apr 21 14:46:33 PDT 2015


Author: rsmith
Date: Tue Apr 21 16:46:32 2015
New Revision: 235420

URL: http://llvm.org/viewvc/llvm-project?rev=235420&view=rev
Log:
[modules] Move list of exported module macros from IdentifierInfo lookup table to separate storage, adjacent to the macro directive history.

This is substantially simpler, provides better space usage accounting in bcanalyzer,
and gives a more compact representation. No functionality change intended.

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

Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=235420&r1=235419&r2=235420&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Tue Apr 21 16:46:32 2015
@@ -600,7 +600,11 @@ namespace clang {
       PP_TOKEN = 3,
 
       /// \brief The macro directives history for a particular identifier.
-      PP_MACRO_DIRECTIVE_HISTORY = 4
+      PP_MACRO_DIRECTIVE_HISTORY = 4,
+
+      /// \brief A macro directive exported by a module.
+      /// [PP_MODULE_MACRO, SubmoduleID, MacroID, (Overridden SubmoduleID)*]
+      PP_MODULE_MACRO = 5,
     };
 
     /// \brief Record types used within a preprocessor detail block.

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=235420&r1=235419&r2=235420&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Apr 21 16:46:32 2015
@@ -676,30 +676,10 @@ private:
 
   struct PendingMacroInfo {
     ModuleFile *M;
+    uint64_t MacroDirectivesOffset;
 
-    struct ModuleMacroDataTy {
-      uint32_t MacID;
-      serialization::SubmoduleID *Overrides;
-    };
-    struct PCHMacroDataTy {
-      uint64_t MacroDirectivesOffset;
-    };
-
-    union {
-      ModuleMacroDataTy ModuleMacroData;
-      PCHMacroDataTy PCHMacroData;
-    };
-
-    PendingMacroInfo(ModuleFile *M,
-                     uint32_t MacID,
-                     serialization::SubmoduleID *Overrides) : M(M) {
-      ModuleMacroData.MacID = MacID;
-      ModuleMacroData.Overrides = Overrides;
-    }
-
-    PendingMacroInfo(ModuleFile *M, uint64_t MacroDirectivesOffset) : M(M) {
-      PCHMacroData.MacroDirectivesOffset = MacroDirectivesOffset;
-    }
+    PendingMacroInfo(ModuleFile *M, uint64_t MacroDirectivesOffset)
+        : M(M), MacroDirectivesOffset(MacroDirectivesOffset) {}
   };
 
   typedef llvm::MapVector<IdentifierInfo *, SmallVector<PendingMacroInfo, 2> >
@@ -1868,14 +1848,8 @@ public:
   serialization::IdentifierID getGlobalIdentifierID(ModuleFile &M,
                                                     unsigned LocalID);
 
-  ModuleMacroInfo *getModuleMacro(IdentifierInfo *II,
-                                  const PendingMacroInfo &PMInfo);
-
   void resolvePendingMacro(IdentifierInfo *II, const PendingMacroInfo &PMInfo);
 
-  void installPCHMacroDirectives(IdentifierInfo *II,
-                                 ModuleFile &M, uint64_t Offset);
-
   void installImportedMacro(IdentifierInfo *II, ModuleMacroInfo *MMI,
                             Module *Owner);
 
@@ -2073,24 +2047,14 @@ public:
   serialization::PreprocessedEntityID
   getGlobalPreprocessedEntityID(ModuleFile &M, unsigned LocalID) const;
 
-  /// \brief Add a macro to resolve imported from a module.
-  ///
-  /// \param II The name of the macro.
-  /// \param M The module file.
-  /// \param GMacID The global macro ID that is associated with this identifier.
-  void addPendingMacroFromModule(IdentifierInfo *II,
-                                 ModuleFile *M,
-                                 serialization::GlobalMacroID GMacID,
-                                 ArrayRef<serialization::SubmoduleID>);
-
-  /// \brief Add a macro to deserialize its macro directive history from a PCH.
+  /// \brief Add a macro to deserialize its macro directive history.
   ///
   /// \param II The name of the macro.
   /// \param M The module file.
   /// \param MacroDirectivesOffset Offset of the serialized macro directive
   /// history.
-  void addPendingMacroFromPCH(IdentifierInfo *II,
-                              ModuleFile *M, uint64_t MacroDirectivesOffset);
+  void addPendingMacro(IdentifierInfo *II, ModuleFile *M,
+                       uint64_t MacroDirectivesOffset);
 
   /// \brief Read the set of macros defined by this external macro source.
   void ReadDefinedMacros() override;

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=235420&r1=235419&r2=235420&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Apr 21 16:46:32 2015
@@ -777,8 +777,6 @@ IdentifierInfo *ASTIdentifierLookupTrait
   Bits >>= 1;
   bool ExtensionToken = Bits & 0x01;
   Bits >>= 1;
-  bool hasSubmoduleMacros = Bits & 0x01;
-  Bits >>= 1;
   bool hadMacroDefinition = Bits & 0x01;
   Bits >>= 1;
 
@@ -820,49 +818,8 @@ IdentifierInfo *ASTIdentifierLookupTrait
     uint32_t MacroDirectivesOffset =
         endian::readNext<uint32_t, little, unaligned>(d);
     DataLen -= 4;
-    SmallVector<uint32_t, 8> LocalMacroIDs;
-    if (hasSubmoduleMacros) {
-      while (true) {
-        uint32_t LocalMacroID =
-            endian::readNext<uint32_t, little, unaligned>(d);
-        DataLen -= 4;
-        if (LocalMacroID == (uint32_t)-1) break;
-        LocalMacroIDs.push_back(LocalMacroID);
-      }
-    }
-
-    if (F.Kind == MK_ImplicitModule || F.Kind == MK_ExplicitModule) {
-      // Macro definitions are stored from newest to oldest, so reverse them
-      // before registering them.
-      llvm::SmallVector<unsigned, 8> MacroSizes;
-      for (SmallVectorImpl<uint32_t>::iterator
-             I = LocalMacroIDs.begin(), E = LocalMacroIDs.end(); I != E; /**/) {
-        unsigned Size = 1;
-
-        static const uint32_t HasOverridesFlag = 0x80000000U;
-        if (I + 1 != E && (I[1] & HasOverridesFlag))
-          Size += 1 + (I[1] & ~HasOverridesFlag);
 
-        MacroSizes.push_back(Size);
-        I += Size;
-      }
-
-      SmallVectorImpl<uint32_t>::iterator I = LocalMacroIDs.end();
-      for (SmallVectorImpl<unsigned>::reverse_iterator SI = MacroSizes.rbegin(),
-                                                       SE = MacroSizes.rend();
-           SI != SE; ++SI) {
-        I -= *SI;
-
-        uint32_t LocalMacroID = *I;
-        ArrayRef<uint32_t> Overrides;
-        if (*SI != 1)
-          Overrides = llvm::makeArrayRef(&I[2], *SI - 2);
-        Reader.addPendingMacroFromModule(II, &F, LocalMacroID, Overrides);
-      }
-      assert(I == LocalMacroIDs.begin());
-    } else {
-      Reader.addPendingMacroFromPCH(II, &F, MacroDirectivesOffset);
-    }
+    Reader.addPendingMacro(II, &F, MacroDirectivesOffset);
   }
 
   Reader.SetIdentifierInfo(ID, II);
@@ -1426,6 +1383,7 @@ MacroInfo *ASTReader::ReadMacroRecord(Mo
     PreprocessorRecordTypes RecType =
       (PreprocessorRecordTypes)Stream.readRecord(Entry.ID, Record);
     switch (RecType) {
+    case PP_MODULE_MACRO:
     case PP_MACRO_DIRECTIVE_HISTORY:
       return Macro;
 
@@ -1619,24 +1577,9 @@ HeaderFileInfoTrait::ReadData(internal_k
   return HFI;
 }
 
-void
-ASTReader::addPendingMacroFromModule(IdentifierInfo *II, ModuleFile *M,
-                                     GlobalMacroID GMacID,
-                                     ArrayRef<SubmoduleID> Overrides) {
-  assert(NumCurrentElementsDeserializing > 0 &&"Missing deserialization guard");
-  SubmoduleID *OverrideData = nullptr;
-  if (!Overrides.empty()) {
-    OverrideData = new (Context) SubmoduleID[Overrides.size() + 1];
-    OverrideData[0] = Overrides.size();
-    for (unsigned I = 0; I != Overrides.size(); ++I)
-      OverrideData[I + 1] = getGlobalSubmoduleID(*M, Overrides[I]);
-  }
-  PendingMacroIDs[II].push_back(PendingMacroInfo(M, GMacID, OverrideData));
-}
-
-void ASTReader::addPendingMacroFromPCH(IdentifierInfo *II,
-                                       ModuleFile *M,
-                                       uint64_t MacroDirectivesOffset) {
+void ASTReader::addPendingMacro(IdentifierInfo *II,
+                                ModuleFile *M,
+                                uint64_t MacroDirectivesOffset) {
   assert(NumCurrentElementsDeserializing > 0 &&"Missing deserialization guard");
   PendingMacroIDs[II].push_back(PendingMacroInfo(M, MacroDirectivesOffset));
 }
@@ -1783,7 +1726,7 @@ void ASTReader::markIdentifierUpToDate(I
 struct ASTReader::ModuleMacroInfo {
   SubmoduleID SubModID;
   MacroInfo *MI;
-  SubmoduleID *Overrides;
+  ArrayRef<SubmoduleID> Overrides;
   // FIXME: Remove this.
   ModuleFile *F;
 
@@ -1791,11 +1734,7 @@ struct ASTReader::ModuleMacroInfo {
 
   SubmoduleID getSubmoduleID() const { return SubModID; }
 
-  ArrayRef<SubmoduleID> getOverriddenSubmodules() const {
-    if (!Overrides)
-      return None;
-    return llvm::makeArrayRef(Overrides + 1, *Overrides);
-  }
+  ArrayRef<SubmoduleID> getOverriddenSubmodules() const { return Overrides; }
 
   MacroDirective *import(Preprocessor &PP, SourceLocation ImportLoc) const {
     if (!MI)
@@ -1806,90 +1745,90 @@ struct ASTReader::ModuleMacroInfo {
   }
 };
 
-ASTReader::ModuleMacroInfo *
-ASTReader::getModuleMacro(IdentifierInfo *II, const PendingMacroInfo &PMInfo) {
-  ModuleMacroInfo Info;
-
-  uint32_t ID = PMInfo.ModuleMacroData.MacID;
-  if (ID & 1) {
-    // Macro undefinition.
-    Info.SubModID = getGlobalSubmoduleID(*PMInfo.M, ID >> 1);
-    Info.MI = nullptr;
-
-    // If we've already loaded the #undef of this macro from this module,
-    // don't do so again.
-    if (!LoadedUndefs.insert(std::make_pair(II, Info.SubModID)).second)
-      return nullptr;
-  } else {
-    // Macro definition.
-    GlobalMacroID GMacID = getGlobalMacroID(*PMInfo.M, ID >> 1);
-    assert(GMacID);
-
-    // If this macro has already been loaded, don't do so again.
-    // FIXME: This is highly dubious. Multiple macro definitions can have the
-    // same MacroInfo (and hence the same GMacID) due to #pragma push_macro etc.
-    if (MacrosLoaded[GMacID - NUM_PREDEF_MACRO_IDS])
-      return nullptr;
-
-    Info.MI = getMacro(GMacID);
-    Info.SubModID = Info.MI->getOwningModuleID();
-  }
-  Info.Overrides = PMInfo.ModuleMacroData.Overrides;
-  Info.F = PMInfo.M;
-
-  return new (Context) ModuleMacroInfo(Info);
-}
-
 void ASTReader::resolvePendingMacro(IdentifierInfo *II,
                                     const PendingMacroInfo &PMInfo) {
-  assert(II);
+  ModuleFile &M = *PMInfo.M;
 
-  if (PMInfo.M->Kind != MK_ImplicitModule &&
-      PMInfo.M->Kind != MK_ExplicitModule) {
-    installPCHMacroDirectives(II, *PMInfo.M,
-                              PMInfo.PCHMacroData.MacroDirectivesOffset);
-    return;
-  }
+  BitstreamCursor &Cursor = M.MacroCursor;
+  SavedStreamPosition SavedPosition(Cursor);
+  Cursor.JumpToBit(PMInfo.MacroDirectivesOffset);
 
-  // Module Macro.
+  llvm::SmallVector<ModuleMacroInfo *, 8> ModuleMacros;
 
-  ModuleMacroInfo *MMI = getModuleMacro(II, PMInfo);
-  if (!MMI)
-    return;
+  // We expect to see a sequence of PP_MODULE_MACRO records listing exported
+  // macros, followed by a PP_MACRO_DIRECTIVE_HISTORY record with the complete
+  // macro histroy.
+  RecordData Record;
+  while (true) {
+    llvm::BitstreamEntry Entry =
+        Cursor.advance(BitstreamCursor::AF_DontPopBlockAtEnd);
+    if (Entry.Kind != llvm::BitstreamEntry::Record) {
+      Error("malformed block record in AST file");
+      return;
+    }
 
-  Module *Owner = getSubmodule(MMI->getSubmoduleID());
-  if (Owner && Owner->NameVisibility == Module::Hidden) {
-    // Macros in the owning module are hidden. Just remember this macro to
-    // install if we make this module visible.
-    HiddenNamesMap[Owner].HiddenMacros.insert(std::make_pair(II, MMI));
-  } else {
-    installImportedMacro(II, MMI, Owner);
-  }
-}
+    Record.clear();
+    switch (PreprocessorRecordTypes RecType =
+                (PreprocessorRecordTypes)Cursor.readRecord(Entry.ID, Record)) {
+    case PP_MACRO_DIRECTIVE_HISTORY:
+      break;
 
-void ASTReader::installPCHMacroDirectives(IdentifierInfo *II,
-                                          ModuleFile &M, uint64_t Offset) {
-  assert(M.Kind != MK_ImplicitModule && M.Kind != MK_ExplicitModule);
+    case PP_MODULE_MACRO: {
+      auto SubModID = getGlobalSubmoduleID(M, Record[0]);
+      auto MacID = getGlobalMacroID(M, Record[1]);
+
+      // Check whether we've already loaded this module macro.
+      // FIXME: The MacrosLoaded check is wrong: multiple macro definitions can
+      // have the same MacroInfo (and the same MacID) due to #pragma pop_macro.
+      if (MacID ? (bool)MacrosLoaded[MacID - NUM_PREDEF_MACRO_IDS]
+                : !LoadedUndefs.insert(std::make_pair(II, SubModID)).second)
+        continue;
+
+      ModuleMacroInfo Info;
+      Info.SubModID = SubModID;
+      Info.MI = getMacro(MacID);
+      Info.F = &M;
+
+      if (Record.size() > 2) {
+        auto *Overrides = new (Context) SubmoduleID[Record.size() - 2];
+        for (int I = 2, N = Record.size(); I != N; ++I)
+          Overrides[I - 2] = getGlobalSubmoduleID(M, Record[I]);
+        Info.Overrides =
+            llvm::makeArrayRef(Overrides, Overrides + Record.size() - 2);
+      }
 
-  BitstreamCursor &Cursor = M.MacroCursor;
-  SavedStreamPosition SavedPosition(Cursor);
-  Cursor.JumpToBit(Offset);
+      ModuleMacros.push_back(new (Context) ModuleMacroInfo(Info));
+      continue;
+    }
 
-  llvm::BitstreamEntry Entry =
-      Cursor.advance(BitstreamCursor::AF_DontPopBlockAtEnd);
-  if (Entry.Kind != llvm::BitstreamEntry::Record) {
-    Error("malformed block record in AST file");
-    return;
+    default:
+      Error("malformed block record in AST file");
+      return;
+    }
+
+    // We found the macro directive history; that's the last record
+    // for this macro.
+    break;
   }
 
-  RecordData Record;
-  PreprocessorRecordTypes RecType =
-    (PreprocessorRecordTypes)Cursor.readRecord(Entry.ID, Record);
-  if (RecType != PP_MACRO_DIRECTIVE_HISTORY) {
-    Error("malformed block record in AST file");
-    return;
+  // Module macros are listed in reverse dependency order.
+  std::reverse(ModuleMacros.begin(), ModuleMacros.end());
+  for (auto *MMI : ModuleMacros) {
+    Module *Owner = getSubmodule(MMI->getSubmoduleID());
+    if (Owner && Owner->NameVisibility == Module::Hidden) {
+      // Macros in the owning module are hidden. Just remember this macro to
+      // install if we make this module visible.
+      HiddenNamesMap[Owner].HiddenMacros.insert(std::make_pair(II, MMI));
+    } else {
+      installImportedMacro(II, MMI, Owner);
+    }
   }
 
+  // Don't read the directive history for a module; we don't have anywhere
+  // to put it.
+  if (M.Kind == MK_ImplicitModule || M.Kind == MK_ExplicitModule)
+    return;
+
   // Deserialize the macro directives history in reverse source-order.
   MacroDirective *Latest = nullptr, *Earliest = nullptr;
   unsigned Idx = 0, N = Record.size();
@@ -1901,12 +1840,14 @@ void ASTReader::installPCHMacroDirective
     case MacroDirective::MD_Define: {
       GlobalMacroID GMacID = getGlobalMacroID(M, Record[Idx++]);
       MacroInfo *MI = getMacro(GMacID);
-      SubmoduleID ImportedFrom = Record[Idx++];
+      SubmoduleID ImportedFrom = getGlobalSubmoduleID(M, Record[Idx++]);
       bool IsAmbiguous = Record[Idx++];
       llvm::SmallVector<unsigned, 4> Overrides;
       if (ImportedFrom) {
         Overrides.insert(Overrides.end(),
                          &Record[Idx] + 1, &Record[Idx] + 1 + Record[Idx]);
+        for (auto &ID : Overrides)
+          ID = getGlobalSubmoduleID(M, ID);
         Idx += Overrides.size() + 1;
       }
       DefMacroDirective *DefMD =
@@ -1916,11 +1857,13 @@ void ASTReader::installPCHMacroDirective
       break;
     }
     case MacroDirective::MD_Undefine: {
-      SubmoduleID ImportedFrom = Record[Idx++];
+      SubmoduleID ImportedFrom = getGlobalSubmoduleID(M, Record[Idx++]);
       llvm::SmallVector<unsigned, 4> Overrides;
       if (ImportedFrom) {
         Overrides.insert(Overrides.end(),
                          &Record[Idx] + 1, &Record[Idx] + 1 + Record[Idx]);
+        for (auto &ID : Overrides)
+          ID = getGlobalSubmoduleID(M, ID);
         Idx += Overrides.size() + 1;
       }
       MD = PP.AllocateUndefMacroDirective(Loc, ImportedFrom, Overrides);

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=235420&r1=235419&r2=235420&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Apr 21 16:46:32 2015
@@ -940,8 +940,9 @@ void ASTWriter::WriteBlockInfoBlock() {
   // Preprocessor Block.
   BLOCK(PREPROCESSOR_BLOCK);
   RECORD(PP_MACRO_DIRECTIVE_HISTORY);
-  RECORD(PP_MACRO_OBJECT_LIKE);
   RECORD(PP_MACRO_FUNCTION_LIKE);
+  RECORD(PP_MACRO_OBJECT_LIKE);
+  RECORD(PP_MODULE_MACRO);
   RECORD(PP_TOKEN);
 
   // Decls and Types block.
@@ -1971,46 +1972,6 @@ void ASTWriter::WriteSourceManagerBlock(
 // Preprocessor Serialization
 //===----------------------------------------------------------------------===//
 
-namespace {
-class ASTMacroTableTrait {
-public:
-  typedef IdentID key_type;
-  typedef key_type key_type_ref;
-
-  struct Data {
-    uint32_t MacroDirectivesOffset;
-  };
-
-  typedef Data data_type;
-  typedef const data_type &data_type_ref;
-  typedef unsigned hash_value_type;
-  typedef unsigned offset_type;
-
-  static hash_value_type ComputeHash(IdentID IdID) {
-    return llvm::hash_value(IdID);
-  }
-
-  std::pair<unsigned,unsigned>
-  static EmitKeyDataLength(raw_ostream& Out,
-                           key_type_ref Key, data_type_ref Data) {
-    unsigned KeyLen = 4; // IdentID.
-    unsigned DataLen = 4; // MacroDirectivesOffset.
-    return std::make_pair(KeyLen, DataLen);
-  }
-
-  static void EmitKey(raw_ostream& Out, key_type_ref Key, unsigned KeyLen) {
-    using namespace llvm::support;
-    endian::Writer<little>(Out).write<uint32_t>(Key);
-  }
-
-  static void EmitData(raw_ostream& Out, key_type_ref Key, data_type_ref Data,
-                       unsigned) {
-    using namespace llvm::support;
-    endian::Writer<little>(Out).write<uint32_t>(Data.MacroDirectivesOffset);
-  }
-};
-} // end anonymous namespace
-
 static bool shouldIgnoreMacro(MacroDirective *MD, bool IsModule,
                               const Preprocessor &PP) {
   if (MacroInfo *MI = MD->getMacroInfo())
@@ -2032,6 +1993,69 @@ static bool shouldIgnoreMacro(MacroDirec
   return false;
 }
 
+static void addOverriddenSubmodules(ASTWriter &Writer, const Preprocessor &PP,
+                                    MacroDirective *MD,
+                                    SmallVectorImpl<uint64_t> &Result) {
+  assert(!isa<VisibilityMacroDirective>(MD) &&
+         "only #define and #undef can override");
+
+  if (MD->isImported()) {
+    auto ModIDs = MD->getOverriddenModules();
+    Result.insert(Result.end(), ModIDs.begin(), ModIDs.end());
+    return;
+  }
+
+  unsigned Start = Result.size();
+
+  SubmoduleID ModID = Writer.inferSubmoduleIDFromLocation(MD->getLocation());
+  for (MD = MD->getPrevious(); MD; MD = MD->getPrevious()) {
+    if (shouldIgnoreMacro(MD, /*IsModule*/true, PP))
+      break;
+
+    // If this is a definition from a submodule import, that submodule's
+    // definition is overridden by the definition or undefinition that we
+    // started with.
+    if (MD->isImported()) {
+      if (auto *DefMD = dyn_cast<DefMacroDirective>(MD)) {
+        SubmoduleID DefModuleID = DefMD->getInfo()->getOwningModuleID();
+        assert(DefModuleID && "imported macro has no owning module");
+        Result.push_back(DefModuleID);
+      } else if (auto *UndefMD = dyn_cast<UndefMacroDirective>(MD)) {
+        // If we override a #undef, we override anything that #undef overrides.
+        // We don't need to override it, since an active #undef doesn't affect
+        // the meaning of a macro.
+        // FIXME: Overriding the #undef directly might be simpler.
+        auto Overrides = UndefMD->getOverriddenModules();
+        Result.insert(Result.end(), Overrides.begin(), Overrides.end());
+      }
+    }
+
+    // Stop once we leave the original macro's submodule.
+    //
+    // Either this submodule #included another submodule of the same
+    // module or it just happened to be built after the other module.
+    // In the former case, we override the submodule's macro.
+    //
+    // FIXME: In the latter case, we shouldn't do so, but we can't tell
+    // these cases apart.
+    //
+    // FIXME: We can leave this submodule and re-enter it if it #includes a
+    // header within a different submodule of the same module. In such cases
+    // the overrides list will be incomplete.
+    SubmoduleID DirectiveModuleID =
+        Writer.inferSubmoduleIDFromLocation(MD->getLocation());
+    if (DirectiveModuleID != ModID) {
+      if (DirectiveModuleID && !MD->isImported())
+        Result.push_back(DirectiveModuleID);
+      break;
+    }
+  }
+
+  // Weed out any duplicate overrides.
+  std::sort(Result.begin() + Start, Result.end());
+  Result.erase(std::unique(Result.begin() + Start, Result.end()), Result.end());
+}
+
 /// \brief Writes the block containing the serialized form of the
 /// preprocessor.
 ///
@@ -2041,6 +2065,7 @@ void ASTWriter::WritePreprocessor(const
     WritePreprocessorDetail(*PPRec);
 
   RecordData Record;
+  RecordData ModuleMacroRecord;
 
   // If the preprocessor __COUNTER__ value has been bumped, remember it.
   if (PP.getCounterValue() != 0) {
@@ -2092,10 +2117,26 @@ void ASTWriter::WritePreprocessor(const
         Name->isFromAST() && !Name->hasChangedSinceDeserialization())
       continue;
 
+    // State of this macro within each submodule.
+    enum class SubmoduleMacroState {
+      /// We've seen nothing about this macro.
+      None,
+      /// We've seen a public visibility directive.
+      Public,
+      /// We've either exported a macro for this module or found that the
+      /// module's definition of this macro is private.
+      Done
+    };
+    llvm::DenseMap<SubmoduleID, SubmoduleMacroState> State;
+
+    auto StartOffset = Stream.GetCurrentBitNo();
+
     // Emit the macro directives in reverse source order.
     for (; MD; MD = MD->getPrevious()) {
+      // Once we hit an ignored macro, we're done: the rest of the chain
+      // will all be ignored macros.
       if (shouldIgnoreMacro(MD, IsModule, PP))
-        continue;
+        break;
 
       AddSourceLocation(MD->getLocation(), Record);
       Record.push_back(MD->getKind());
@@ -2116,11 +2157,46 @@ void ASTWriter::WritePreprocessor(const
         Record.push_back(Overrides.size());
         Record.append(Overrides.begin(), Overrides.end());
       }
+
+      // If this is the final definition in some module, and it's not
+      // module-private, add a module macro record for it.
+      if (IsModule) {
+        SubmoduleID ModID =
+            MD->isImported() ? MD->getOwningModuleID()
+                             : inferSubmoduleIDFromLocation(MD->getLocation());
+        assert(ModID && "found macro in no submodule");
+
+        auto &S = State[ModID];
+        if (S == SubmoduleMacroState::Done) {
+          // We've already handled the final macro from this submodule, or seen
+          // a private visibility directive.
+        } else if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
+          // The latest visibility directive for a name in a submodule affects
+          // all the directives that come before it.
+          if (S == SubmoduleMacroState::None)
+            S = VisMD->isPublic() ? SubmoduleMacroState::Public
+                                  : SubmoduleMacroState::Done;
+        } else {
+          S = SubmoduleMacroState::Done;
+
+          // Emit a record indicating this submodule exports this macro.
+          ModuleMacroRecord.push_back(ModID);
+          if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(MD))
+            ModuleMacroRecord.push_back(getMacroID(DefMD->getInfo()));
+          else
+            ModuleMacroRecord.push_back(0);
+          addOverriddenSubmodules(*this, PP, MD, ModuleMacroRecord);
+
+          Stream.EmitRecord(PP_MODULE_MACRO, ModuleMacroRecord);
+          ModuleMacroRecord.clear();
+        }
+      }
     }
+
     if (Record.empty())
       continue;
 
-    IdentMacroDirectivesOffsetMap[Name] = Stream.GetCurrentBitNo();
+    IdentMacroDirectivesOffsetMap[Name] = StartOffset;
     Stream.EmitRecord(PP_MACRO_DIRECTIVE_HISTORY, Record);
     Record.clear();
   }
@@ -3130,106 +3206,18 @@ static NamedDecl *getDeclForLocalLookup(
 }
 
 namespace {
-class PublicMacroIterator {
-  ASTWriter &Writer;
-  Preprocessor &PP;
-  bool IsModule;
-  MacroDirective *Macro;
-
-  enum class SubmoduleMacroState {
-    /// We've seen nothing about this macro.
-    None,
-    /// We've seen a public visibility directive.
-    Public,
-    /// We've either exported a macro for this module or found that the
-    /// module's definition of this macro is private.
-    Done
-  };
-  llvm::DenseMap<SubmoduleID, SubmoduleMacroState> State;
-
-public:
-  PublicMacroIterator(ASTWriter &Writer, Preprocessor &PP, bool IsModule,
-                      IdentifierInfo *II)
-      : Writer(Writer), PP(PP), IsModule(IsModule), Macro(nullptr) {
-    if (!II->hadMacroDefinition())
-      return;
-
-    Macro = PP.getMacroDirectiveHistory(II);
-
-    if (IsModule)
-      Macro = skipUnexportedMacros(Macro);
-    else if (shouldIgnoreMacro(Macro, IsModule, PP))
-      Macro = nullptr;
-  }
-
-  MacroDirective *operator*() const { return Macro; }
-  PublicMacroIterator &operator++() {
-    Macro = skipUnexportedMacros(Macro->getPrevious());
-    return *this;
-  }
-
-  explicit operator bool() const { return Macro; }
-
-private:
-  /// \brief Traverses the macro directives history and returns the next
-  /// public macro definition or undefinition that has not been found so far.
-  ///
-  /// A macro that is defined in submodule A and undefined in submodule B
-  /// will still be considered as defined/exported from submodule A.
-  MacroDirective *skipUnexportedMacros(MacroDirective *MD) {
-    if (!MD || !IsModule)
-      return nullptr;
-
-    Optional<bool> IsPublic;
-    for (; MD; MD = MD->getPrevious()) {
-      // Once we hit an ignored macro, we're done: the rest of the chain
-      // will all be ignored macros.
-      if (shouldIgnoreMacro(MD, IsModule, PP))
-        break;
-
-      // If this macro was imported, re-export it.
-      if (MD->isImported())
-        return MD;
-
-      SubmoduleID ModID =
-          Writer.inferSubmoduleIDFromLocation(MD->getLocation());
-      auto &S = State[ModID];
-      assert(ModID && "found macro in no submodule");
-
-      if (S == SubmoduleMacroState::Done)
-        continue;
-
-      if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
-        // The latest visibility directive for a name in a submodule affects all
-        // the directives that come before it.
-        if (S == SubmoduleMacroState::None)
-          S = VisMD->isPublic() ? SubmoduleMacroState::Public
-                                : SubmoduleMacroState::Done;
-      } else {
-        S = SubmoduleMacroState::Done;
-        return MD;
-      }
-    }
-
-    return nullptr;
-  }
-};
-
 class ASTIdentifierTableTrait {
   ASTWriter &Writer;
   Preprocessor &PP;
   IdentifierResolver &IdResolver;
-  bool IsModule;
   
   /// \brief Determines whether this is an "interesting" identifier that needs a
   /// full IdentifierInfo structure written into the hash table. Notably, this
   /// doesn't check whether the name has macros defined; use PublicMacroIterator
   /// to check that.
-  bool isInterestingIdentifier(IdentifierInfo *II) {
-    if (PublicMacroIterator(Writer, PP, IsModule, II))
-      return true;
-
-    if (II->isPoisoned() ||
+  bool isInterestingIdentifier(IdentifierInfo *II, uint64_t MacroOffset) {
+    if (MacroOffset ||
+        II->isPoisoned() ||
         II->isExtensionToken() ||
         II->getObjCOrBuiltinID() ||
         II->hasRevertedTokenIDToIdentifier() ||
@@ -3239,68 +3227,6 @@ class ASTIdentifierTableTrait {
     return false;
   }
 
-  ArrayRef<SubmoduleID>
-  getOverriddenSubmodules(MacroDirective *MD,
-                          SmallVectorImpl<SubmoduleID> &ScratchSpace) {
-    assert(!isa<VisibilityMacroDirective>(MD) &&
-           "only #define and #undef can override");
-    if (MD->isImported())
-      return MD->getOverriddenModules();
-
-    ScratchSpace.clear();
-    SubmoduleID ModID = getSubmoduleID(MD);
-    for (MD = MD->getPrevious(); MD; MD = MD->getPrevious()) {
-      if (shouldIgnoreMacro(MD, IsModule, PP))
-        break;
-
-      // If this is a definition from a submodule import, that submodule's
-      // definition is overridden by the definition or undefinition that we
-      // started with.
-      if (MD->isImported()) {
-        if (auto *DefMD = dyn_cast<DefMacroDirective>(MD)) {
-          SubmoduleID DefModuleID = DefMD->getInfo()->getOwningModuleID();
-          assert(DefModuleID && "imported macro has no owning module");
-          ScratchSpace.push_back(DefModuleID);
-        } else if (auto *UndefMD = dyn_cast<UndefMacroDirective>(MD)) {
-          // If we override a #undef, we override anything that #undef overrides.
-          // We don't need to override it, since an active #undef doesn't affect
-          // the meaning of a macro.
-          auto Overrides = UndefMD->getOverriddenModules();
-          ScratchSpace.insert(ScratchSpace.end(),
-                              Overrides.begin(), Overrides.end());
-        }
-      }
-
-      // Stop once we leave the original macro's submodule.
-      //
-      // Either this submodule #included another submodule of the same
-      // module or it just happened to be built after the other module.
-      // In the former case, we override the submodule's macro.
-      //
-      // FIXME: In the latter case, we shouldn't do so, but we can't tell
-      // these cases apart.
-      //
-      // FIXME: We can leave this submodule and re-enter it if it #includes a
-      // header within a different submodule of the same module. In such cases
-      // the overrides list will be incomplete.
-      SubmoduleID DirectiveModuleID = getSubmoduleID(MD);
-      if (DirectiveModuleID != ModID) {
-        if (DirectiveModuleID && !MD->isImported())
-          ScratchSpace.push_back(DirectiveModuleID);
-        break;
-      }
-    }
-
-    std::sort(ScratchSpace.begin(), ScratchSpace.end());
-    ScratchSpace.erase(std::unique(ScratchSpace.begin(), ScratchSpace.end()),
-                       ScratchSpace.end());
-    return ScratchSpace;
-  }
-
-  SubmoduleID getSubmoduleID(MacroDirective *MD) {
-    return Writer.inferSubmoduleIDFromLocation(MD->getLocation());
-  }
-
 public:
   typedef IdentifierInfo* key_type;
   typedef key_type  key_type_ref;
@@ -3311,9 +3237,9 @@ public:
   typedef unsigned hash_value_type;
   typedef unsigned offset_type;
 
-  ASTIdentifierTableTrait(ASTWriter &Writer, Preprocessor &PP, 
-                          IdentifierResolver &IdResolver, bool IsModule)
-    : Writer(Writer), PP(PP), IdResolver(IdResolver), IsModule(IsModule) { }
+  ASTIdentifierTableTrait(ASTWriter &Writer, Preprocessor &PP,
+                          IdentifierResolver &IdResolver)
+      : Writer(Writer), PP(PP), IdResolver(IdResolver) {}
 
   static hash_value_type ComputeHash(const IdentifierInfo* II) {
     return llvm::HashString(II->getName());
@@ -3323,23 +3249,12 @@ public:
   EmitKeyDataLength(raw_ostream& Out, IdentifierInfo* II, IdentID ID) {
     unsigned KeyLen = II->getLength() + 1;
     unsigned DataLen = 4; // 4 bytes for the persistent ID << 1
-    PublicMacroIterator Macros(Writer, PP, IsModule, II);
-    if (Macros || isInterestingIdentifier(II)) {
+    auto MacroOffset = Writer.getMacroDirectivesOffset(II);
+    if (isInterestingIdentifier(II, MacroOffset)) {
       DataLen += 2; // 2 bytes for builtin ID
       DataLen += 2; // 2 bytes for flags
-      if (Macros) {
+      if (MacroOffset)
         DataLen += 4; // MacroDirectives offset.
-        if (IsModule) {
-          SmallVector<SubmoduleID, 16> Scratch;
-          for (; Macros; ++Macros) {
-            DataLen += 4; // MacroInfo ID or ModuleID.
-            if (unsigned NumOverrides =
-                    getOverriddenSubmodules(*Macros, Scratch).size())
-              DataLen += 4 * (1 + NumOverrides);
-          }
-          DataLen += 4; // 0 terminator.
-        }
-      }
 
       for (IdentifierResolver::iterator D = IdResolver.begin(II),
                                      DEnd = IdResolver.end();
@@ -3366,26 +3281,13 @@ public:
     Out.write(II->getNameStart(), KeyLen);
   }
 
-  static void emitMacroOverrides(raw_ostream &Out,
-                                 ArrayRef<SubmoduleID> Overridden) {
-    if (!Overridden.empty()) {
-      using namespace llvm::support;
-      endian::Writer<little> LE(Out);
-      LE.write<uint32_t>(Overridden.size() | 0x80000000U);
-      for (unsigned I = 0, N = Overridden.size(); I != N; ++I) {
-        assert(Overridden[I] && "zero module ID for override");
-        LE.write<uint32_t>(Overridden[I]);
-      }
-    }
-  }
-
   void EmitData(raw_ostream& Out, IdentifierInfo* II,
                 IdentID ID, unsigned) {
     using namespace llvm::support;
     endian::Writer<little> LE(Out);
 
-    PublicMacroIterator Macros(Writer, PP, IsModule, II);
-    if (!Macros && !isInterestingIdentifier(II)) {
+    auto MacroOffset = Writer.getMacroDirectivesOffset(II);
+    if (!isInterestingIdentifier(II, MacroOffset)) {
       LE.write<uint32_t>(ID << 1);
       return;
     }
@@ -3395,41 +3297,16 @@ public:
     assert((Bits & 0xffff) == Bits && "ObjCOrBuiltinID too big for ASTReader.");
     LE.write<uint16_t>(Bits);
     Bits = 0;
-    bool HadMacroDefinition = !!Macros;
+    bool HadMacroDefinition = MacroOffset != 0;
     Bits = (Bits << 1) | unsigned(HadMacroDefinition);
-    Bits = (Bits << 1) | unsigned(IsModule);
     Bits = (Bits << 1) | unsigned(II->isExtensionToken());
     Bits = (Bits << 1) | unsigned(II->isPoisoned());
     Bits = (Bits << 1) | unsigned(II->hasRevertedTokenIDToIdentifier());
     Bits = (Bits << 1) | unsigned(II->isCPlusPlusOperatorKeyword());
     LE.write<uint16_t>(Bits);
 
-    if (HadMacroDefinition) {
-      LE.write<uint32_t>(Writer.getMacroDirectivesOffset(II));
-      if (IsModule) {
-        // Write the IDs of macros coming from different submodules.
-        SmallVector<SubmoduleID, 16> Scratch;
-        for (; Macros; ++Macros) {
-          if (DefMacroDirective *DefMD = dyn_cast<DefMacroDirective>(*Macros)) {
-            // FIXME: If this macro directive was created by #pragma pop_macros,
-            // or if it was created implicitly by resolving conflicting macros,
-            // it may be for a different submodule from the one in the MacroInfo
-            // object. If so, we should write out its owning ModuleID.
-            MacroID InfoID = Writer.getMacroID(DefMD->getInfo());
-            assert(InfoID);
-            LE.write<uint32_t>(InfoID << 1);
-          } else {
-            auto *UndefMD = cast<UndefMacroDirective>(*Macros);
-            SubmoduleID Mod = UndefMD->isImported()
-                                  ? UndefMD->getOwningModuleID()
-                                  : getSubmoduleID(UndefMD);
-            LE.write<uint32_t>((Mod << 1) | 1);
-          }
-          emitMacroOverrides(Out, getOverriddenSubmodules(*Macros, Scratch));
-        }
-        LE.write<uint32_t>((uint32_t)-1);
-      }
-    }
+    if (HadMacroDefinition)
+      LE.write<uint32_t>(MacroOffset);
 
     // Emit the declaration IDs in reverse order, because the
     // IdentifierResolver provides the declarations as they would be
@@ -3461,7 +3338,7 @@ void ASTWriter::WriteIdentifierTable(Pre
   // strings.
   {
     llvm::OnDiskChainedHashTableGenerator<ASTIdentifierTableTrait> Generator;
-    ASTIdentifierTableTrait Trait(*this, PP, IdResolver, IsModule);
+    ASTIdentifierTableTrait Trait(*this, PP, IdResolver);
 
     // Look for any identifiers that were named while processing the
     // headers, but are otherwise not needed. We add these to the hash
@@ -3495,7 +3372,6 @@ void ASTWriter::WriteIdentifierTable(Pre
     uint32_t BucketOffset;
     {
       using namespace llvm::support;
-      ASTIdentifierTableTrait Trait(*this, PP, IdResolver, IsModule);
       llvm::raw_svector_ostream Out(IdentifierTable);
       // Make sure that no bucket is at offset 0
       endian::Writer<little>(Out).write<uint32_t>(0);
@@ -4941,8 +4817,7 @@ MacroID ASTWriter::getMacroID(MacroInfo
 }
 
 uint64_t ASTWriter::getMacroDirectivesOffset(const IdentifierInfo *Name) {
-  assert(IdentMacroDirectivesOffsetMap[Name] && "not set!");
-  return IdentMacroDirectivesOffsetMap[Name];
+  return IdentMacroDirectivesOffsetMap.lookup(Name);
 }
 
 void ASTWriter::AddSelectorRef(const Selector SelRef, RecordDataImpl &Record) {





More information about the cfe-commits mailing list