Keep history of macro definitions and #undefs

[alexfh]

Summary

Summary: Keep history of macro definitions and #undefs with corresponding source locations, so that we can later find out all macros active in a specified source location. We don't save the history in PCH (no need currently). Memory overhead is about sizeof(void*)*3*<number of macro definitions and #undefs>+<in-memory size of all #undef'd macros>

I've run a test on a file composed of 109 .h files from boost 1.49 on x86-64 linux.
Stats before this patch:

• Preprocessor Stats:

73222 directives found:

19171 #define.
4345 #undef.
#include/#include_next/#import:
  5233 source files entered.
  27 max include stack depth
19210 #if/#ifndef/#ifdef.
2384 #else/#elif.
6891 #endif.
408 #pragma.

14466 #if/#ifndef#ifdef regions skipped
80023/451669/1270 obj/fn/builtin macros expanded, 85724 on the fast path.
127145 token paste (##) operations performed, 11008 on the fast path.

Preprocessor Memory: 5874615B total

BumpPtr: 4399104
Macro Expanded Tokens: 417768
Predefines Buffer: 8135
Macros: 1048576
#pragma push_macro Info: 0
Poison Reasons: 1024
Comment Handlers: 8

Stats with this patch:
...
Preprocessor Memory: 7541687B total

BumpPtr: 6066176
Macro Expanded Tokens: 417768
Predefines Buffer: 8135
Macros: 1048576
#pragma push_macro Info: 0
Poison Reasons: 1024
Comment Handlers: 8

In my test increase in memory usage is about 1.7Mb, which is ~28% of initial preprocessor's memory usage and about 0.8% of clang's total VMM allocation.

As for CPU overhead, it should only be noticeable when iterating over all macros, and should mostly consist of couple extra dereferences and one comparison per macro + skipping of #undef'd macros. It's less trivial to measure, though, as the preprocessor consumes a very small fraction of compilation time.

[alexfh]

more context lines in diff

[doug.gregor]

Thanks for working on this! Some comments inline.

tools/clang/include/clang/Lex/Preprocessor.h

476

Why do we need the definition location in the MacroInfoList? For #defines, the location is already in the MacroInfo. For #undefs, it would seem to be more valuable to simply model the #undef as a

SourceLocation UndefLocation;

within the MacroInfo itself. An active macro will have an invalid UndefLocation.

478

This is really a link to the previously-defined macro, right?

tools/clang/lib/Lex/PPMacroExpansion.cpp

62

llvm_unreachable?

tools/clang/lib/Sema/SemaCodeComplete.cpp

2907

Eventually, we'd want to be able to look back to the macro definition that was actually active at the point of code completion (even if that macro has since been #undef'd). Please add a FIXME for this.

7174

This is a code completion after, e.g., #ifdef, so any macro name we know about---even if it's been #undef'd---is completely reasonable here.

tools/clang/lib/Serialization/ASTWriter.cpp

1679

We need to store the macro history, because whether one uses a PCH file should impact only the performance of the front end, not the actual meaning of the compiler's data structures.

The (currently stalled, but ongoing) work on modules actually needs this macro history information, too.

[alexfh]

Sending the new patch right after this comment.

tools/clang/include/clang/Lex/Preprocessor.h

476

This serves as bot #define and #undef location. I think this should simplify code using this structure. But maybe the name would better be just 'Location' or 'DefUndefLocation' or something like that. What do you think?

478

It's 'Next' in terms of linked list, but I could make it 'Previous' if you like ;)

tools/clang/lib/Lex/PPMacroExpansion.cpp

62

Yep. Done.

tools/clang/lib/Sema/SemaCodeComplete.cpp

2907

Done.

7174

Reverted this.

tools/clang/lib/Serialization/ASTWriter.cpp

1679

Added FIXME.

[alexfh]

Addressed Doug's comments.

[alexfh]

Ping.

[alexfh]

Ping.

[alexfh]

Removed MacroInfoList, added PreviousDeclaration and UndefLocation to MacroInfo.

[alexfh]

After I posted this patch, I found, that it has a bug with macro redefinition handling. Will fix later today or tomorrow. But, please, still look at other things.

[alexfh]

Fixed issue with pragma push_macro/pop_macro

[chandlerc]

I'll leave the real review to Doug, but I have a tiny style nit. ;]

tools/clang/lib/Frontend/PrintPreprocessedOutput.cpp

571–579

FYI, please use the conventional naming for loop iterators in Clang/LLVM: I and E.

[alexfh]

It, End -> I, E

[doug.gregor]

This is fine. We should tackle (de-)serialization separately.

[alexfh]

I've made a quick-and-dirty performance measurement using almost all boost
1.51 headers. The test source file, script and execution logs are attached.
Clang was built with gcc (hence log names).

TL;DR: changes 'caused by the patch: preprocessor memory usage 5.9M ->7.4M,
total time: no measurable difference (both about 4.8s).

As for the test input, I consider it quite a heavy load on preprocessor:

• Preprocessor Stats:

75165 directives found:

19499 #define.
4433 #undef.
#include/#include_next/#import:
  5357 source files entered.
  27 max include stack depth
19919 #if/#ifndef/#ifdef.
2459 #else/#elif.
7110 #endif.
426 #pragma.

14996 #if/#ifndef#ifdef regions skipped
82955/459701/1316 obj/fn/builtin macros expanded, 88278 on the fast path.
129361 token paste (##) operations performed, 11361 on the fast path.

Difference between execution logs:

diff clang-gcc-perf-before-patch.log clang-gcc-perf-with-patch.log
193,194c193,194
< Preprocessor Memory: 5940151B total

< BumpPtr: 4464640

Preprocessor Memory: 7394231B total

BumpPtr: 5918720

229c229

< FileID scans: 205598 linear, 2102385 binary.

FileID scans: 205604 linear, 2102345 binary.

239,241c239,241
< real 0m4.872s
< user 0m4.700s

< sys 0m0.160s

real 0m4.796s
user 0m4.560s
sys 0m0.220s

434,435c434,435
< Preprocessor Memory: 5940151B total

< BumpPtr: 4464640

Preprocessor Memory: 7394231B total

BumpPtr: 5918720

480,482c480,482
< real 0m4.819s
< user 0m4.620s

< sys 0m0.180s

real 0m4.882s
user 0m4.730s
sys 0m0.140s

675,676c675,676
< Preprocessor Memory: 5940151B total

< BumpPtr: 4464640

Preprocessor Memory: 7394231B total

BumpPtr: 5918720

711c711

< FileID scans: 205598 linear, 2102365 binary.

FileID scans: 205604 linear, 2102385 binary.

721,723c721,723
< real 0m4.794s
< user 0m4.610s

< sys 0m0.170s

real 0m4.789s
user 0m4.630s
sys 0m0.150s

[alexfh]

I've made a quick-and-dirty performance measurement using almost all boost
1.51 headers. The test source file, script and execution logs are attached.
Clang was built with gcc (hence log names).

TL;DR: changes 'caused by the patch: preprocessor memory usage 5.9M ->7.4M,
total time: no measurable difference (both about 4.8s).

As for the test input, I consider it quite a heavy load on preprocessor:

• Preprocessor Stats:

75165 directives found:

19499 #define.
4433 #undef.
#include/#include_next/#import:
  5357 source files entered.
  27 max include stack depth
19919 #if/#ifndef/#ifdef.
2459 #else/#elif.
7110 #endif.
426 #pragma.

14996 #if/#ifndef#ifdef regions skipped
82955/459701/1316 obj/fn/builtin macros expanded, 88278 on the fast path.
129361 token paste (##) operations performed, 11361 on the fast path.

Difference between execution logs:

diff clang-gcc-perf-before-patch.log clang-gcc-perf-with-patch.log
193,194c193,194
< Preprocessor Memory: 5940151B total

< BumpPtr: 4464640

Preprocessor Memory: 7394231B total

BumpPtr: 5918720

229c229

< FileID scans: 205598 linear, 2102385 binary.

FileID scans: 205604 linear, 2102345 binary.

239,241c239,241
< real 0m4.872s
< user 0m4.700s

< sys 0m0.160s

real 0m4.796s
user 0m4.560s
sys 0m0.220s

434,435c434,435
< Preprocessor Memory: 5940151B total

< BumpPtr: 4464640

Preprocessor Memory: 7394231B total

BumpPtr: 5918720

480,482c480,482
< real 0m4.819s
< user 0m4.620s

< sys 0m0.180s

real 0m4.882s
user 0m4.730s
sys 0m0.140s

675,676c675,676
< Preprocessor Memory: 5940151B total

< BumpPtr: 4464640

Preprocessor Memory: 7394231B total

BumpPtr: 5918720

711c711

< FileID scans: 205598 linear, 2102365 binary.

FileID scans: 205604 linear, 2102385 binary.

721,723c721,723
< real 0m4.794s
< user 0m4.610s

< sys 0m0.170s

real 0m4.789s
user 0m4.630s
sys 0m0.150s

tools/clang/include/clang/Lex/Preprocessor.h

struct MacroExpandsInfo {
     Token Tok;
     MacroInfo *MI;
     SourceRange Range;
     MacroExpandsInfo(Token Tok, MacroInfo *MI, SourceRange Range)
       : Tok(Tok), MI(MI), Range(Range) { }
   };
   SmallVector<MacroExpandsInfo, 2> DelayedMacroExpandsCallbacks;
 
-  /// Macros - For each IdentifierInfo with 'HasMacro' set, we keep a mapping
-  /// to the actual definition of the macro.
-  llvm::DenseMap<IdentifierInfo*, MacroInfo*> Macros;
-
   /// \brief Macros that we want to warn because they are not used at the end
   /// of the translation unit; we store just their SourceLocations instead
   /// something like MacroInfo*. The benefit of this is that when we are
   /// deserializing from PCH, we don't need to deserialize identifier & macros
   /// just so that we can report that they are unused, we just warn using
   /// the SourceLocations of this set (that will be filled by the ASTReader).
   /// We are using SmallPtrSet instead of a vector for faster removal.
   typedef llvm::SmallPtrSet<SourceLocation, 32> WarnUnusedMacroLocsTy;
MacroInfo *getMacroInfo(IdentifierInfo *II) const {
     if (!II->hasMacroDefinition())
       return 0;
 
     return getInfoForMacro(II);
   }
 
   /// \brief Specify a macro for this identifier.
   void setMacroInfo(IdentifierInfo *II, MacroInfo *MI,
-                    bool LoadedFromAST = false);
+                    SourceLocation Location, bool LoadedFromAST = false);
+
+  /// Used to keep a history of macro definitions and #undefs with
+  /// corresponding source locations.
+  struct MacroInfoList {
+    /// Actual MacroInfo for definition, NULL for #undef.
+    MacroInfo *MI;
+    /// Location of macro name token in #define or #undef. An empty
+    /// SourceLocation for built-in macros.
+    SourceLocation DefinitionLoc;
+    /// Next node in the list.
+    MacroInfoList *Next;
+
+    MacroInfoList(MacroInfo *MI, SourceLocation DefinitionLoc,
+                  MacroInfoList *Next) :
+      MI(MI), DefinitionLoc(DefinitionLoc), Next(Next) {}
+  };
 
-  /// macro_iterator/macro_begin/macro_end - This allows you to walk the current
-  /// state of the macro table.  This visits every currently-defined macro.
+private:
+  /// Macros - For each IdentifierInfo that was associated with a macro, we
+  /// keep a mapping to the history of all macro definitions and #undefs in
+  /// the reverse order (the latest one is in the head of the list).
+  llvm::DenseMap<IdentifierInfo*, MacroInfoList*> Macros;
+
+public:
+  /// macro_iterator/macro_begin/macro_end - This allows you to walk the macro
+  /// history table. Currently defined macros have
+  /// IdentifierInfo::hasMacroDefinition() set and non-zero MacroInfoList::MI
+  /// at the head of the list.
   typedef llvm::DenseMap<IdentifierInfo*,
-                         MacroInfo*>::const_iterator macro_iterator;
+                         MacroInfoList*>::const_iterator macro_iterator;
   macro_iterator macro_begin(bool IncludeExternalMacros = true) const;
   macro_iterator macro_end(bool IncludeExternalMacros = true) const;
 
   const std::string &getPredefines() const { return Predefines; }
   /// setPredefines - Set the predefines for this Preprocessor.  These
   /// predefines are automatically injected when parsing the main file.
   void setPredefines(const char *P) { Predefines = P; }
   void setPredefines(const std::string &P) { Predefines = P; }

tools/clang/lib/Frontend/PrintPreprocessedOutput.cpp

static void DoPrintMacros(Preprocessor &PP, raw_ostream *OS) {
   PP.AddPragmaHandler(new EmptyPragmaHandler());
 
   // -dM mode just scans and ignores all tokens in the files, then dumps out
   // the macro table at the end.
   PP.EnterMainSourceFile();
 
   Token Tok;
   do PP.Lex(Tok);
   while (Tok.isNot(tok::eof));
 
-  SmallVector<id_macro_pair, 128>
-    MacrosByID(PP.macro_begin(), PP.macro_end());
+  SmallVector<id_macro_pair, 128> MacrosByID;
+  for (Preprocessor::macro_iterator It = PP.macro_begin(), End = PP.macro_end();
+       It != End; ++It) {
+    if (It->second->MI)
+      MacrosByID.push_back(id_macro_pair(It->first, It->second->MI));
+  }
   llvm::array_pod_sort(MacrosByID.begin(), MacrosByID.end(), MacroIDCompare);
 
   for (unsigned i = 0, e = MacrosByID.size(); i != e; ++i) {
     MacroInfo &MI = *MacrosByID[i].second;
     // Ignore computed macros like __LINE__ and friends.
     if (MI.isBuiltinMacro()) continue;
 
     PrintMacroDefinition(*MacrosByID[i].first, MI, PP, *OS);
     *OS << '\n';

tools/clang/lib/Lex/PPDirectives.cpp

if (MI->getReplacementToken(NumTokens-1).is(tok::hashhash)) {
       ReleaseMacroInfo(MI);
       return;
     }
   }
 
   MI->setDefinitionEndLoc(LastTok.getLocation());
 
   // Finally, if this identifier already had a macro defined for it, verify that
-  // the macro bodies are identical and free the old definition.
+  // the macro bodies are identical, and issue diagnostics if they are not.
   if (MacroInfo *OtherMI = getMacroInfo(MacroNameTok.getIdentifierInfo())) {
     // It is very common for system headers to have tons of macro redefinitions
     // and for warnings to be disabled in system headers.  If this is the case,
     // then don't bother calling MacroInfo::isIdenticalTo.
     if (!getDiagnostics().getSuppressSystemWarnings() ||
         !SourceMgr.isInSystemHeader(DefineTok.getLocation())) {
       if (!OtherMI->isUsed() && OtherMI->isWarnIfUnused())
         Diag(OtherMI->getDefinitionLoc(), diag::pp_macro_not_used);
 
       // Macros must be identical.  This means all tokens and whitespace
       // separation must be the same.  C99 6.10.3.2.
       if (!OtherMI->isAllowRedefinitionsWithoutWarning() &&
           !MI->isIdenticalTo(*OtherMI, *this)) {
         Diag(MI->getDefinitionLoc(), diag::ext_pp_macro_redef)
           << MacroNameTok.getIdentifierInfo();
         Diag(OtherMI->getDefinitionLoc(), diag::note_previous_definition);
       }
     }
     if (OtherMI->isWarnIfUnused())
       WarnUnusedMacroLocs.erase(OtherMI->getDefinitionLoc());
-    ReleaseMacroInfo(OtherMI);
   }
 
-  setMacroInfo(MacroNameTok.getIdentifierInfo(), MI);
+  setMacroInfo(MacroNameTok.getIdentifierInfo(), MI,
+               MacroNameTok.getLocation());
 
   assert(!MI->isUsed());
   // If we need warning for not using the macro, add its location in the
   // warn-because-unused-macro set. If it gets used it will be removed from set.
   if (isInPrimaryFile() && // don't warn for include'd macros.
       Diags->getDiagnosticLevel(diag::pp_macro_not_used,
           MI->getDefinitionLoc()) != DiagnosticsEngine::Ignored) {
     MI->setIsWarnIfUnused(true);
void Preprocessor::HandleUndefDirective(Token &UndefTok) {
 
   // If the callbacks want to know, tell them about the macro #undef.
   if (Callbacks)
     Callbacks->MacroUndefined(MacroNameTok, MI);
 
   if (MI->isWarnIfUnused())
     WarnUnusedMacroLocs.erase(MI->getDefinitionLoc());
 
-  // Free macro definition.
-  ReleaseMacroInfo(MI);
-  setMacroInfo(MacroNameTok.getIdentifierInfo(), 0);
+  setMacroInfo(MacroNameTok.getIdentifierInfo(), 0, MacroNameTok.getLocation());
 }
 
 
 //===----------------------------------------------------------------------===//
 // Preprocessor Conditional Directive Handling.
 //===----------------------------------------------------------------------===//
 
 /// HandleIfdefDirective - Implements the \#ifdef/\#ifndef directive.  isIfndef

tools/clang/lib/Lex/PPMacroExpansion.cpp

 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cstdio>
 #include <ctime>
 using namespace clang;
 
 MacroInfo *Preprocessor::getInfoForMacro(IdentifierInfo *II) const {
   assert(II->hasMacroDefinition() && "Identifier is not a macro!");
-  
-  llvm::DenseMap<IdentifierInfo*, MacroInfo*>::const_iterator Pos
-    = Macros.find(II);
+  macro_iterator Pos = Macros.find(II);
   if (Pos == Macros.end()) {
     // Load this macro from the external source.
     getExternalSource()->LoadMacroDefinition(II);
     Pos = Macros.find(II);
   }
   assert(Pos != Macros.end() && "Identifier macro info is missing!");
-  return Pos->second;
+  assert(Pos->second && "MacroInfoList is missing!");
+  assert(Pos->second->MI && "Macro seems to be undefined!");
+  return Pos->second->MI;
 }
 
 /// setMacroInfo - Specify a macro for this identifier.
 ///
 void Preprocessor::setMacroInfo(IdentifierInfo *II, MacroInfo *MI,
-                                bool LoadedFromAST) {
-  if (MI) {
-    Macros[II] = MI;
-    II->setHasMacroDefinition(true);
-    if (II->isFromAST() && !LoadedFromAST)
-      II->setChangedSinceDeserialization();
-  } else if (II->hasMacroDefinition()) {
-    Macros.erase(II);
-    II->setHasMacroDefinition(false);
+                                SourceLocation Location, bool LoadedFromAST) {
+  if (MI || II->hasMacroDefinition()) {
+    MacroInfoList *NewItem = BP.Allocate<MacroInfoList>();
+    *NewItem = MacroInfoList(MI, Location, Macros[II]);
+    Macros[II] = NewItem;
+    II->setHasMacroDefinition(MI != 0);
     if (II->isFromAST() && !LoadedFromAST)
       II->setChangedSinceDeserialization();
+  } else {
+    // This case is handled in the caller
+    assert(0 && "Trying to undefine undefined macro!");
   }
 }
 
 /// RegisterBuiltinMacro - Register the specified identifier in the identifier
 /// table and mark it as a builtin macro to be expanded.
 static IdentifierInfo *RegisterBuiltinMacro(Preprocessor &PP, const char *Name){
   // Get the identifier.
   IdentifierInfo *Id = PP.getIdentifierInfo(Name);
 
   // Mark it as being a macro that is builtin.
   MacroInfo *MI = PP.AllocateMacroInfo(SourceLocation());
   MI->setIsBuiltinMacro();
-  PP.setMacroInfo(Id, MI);
+  PP.setMacroInfo(Id, MI, SourceLocation());
   return Id;
 }
 
 
 /// RegisterBuiltinMacros - Register builtin macros, such as __LINE__ with the
 /// identifier table.
 void Preprocessor::RegisterBuiltinMacros() {
   Ident__LINE__ = RegisterBuiltinMacro(*this, "__LINE__");

tools/clang/lib/Lex/Pragma.cpp

void Preprocessor::HandlePragmaPopMacro(Token &PopMacroTok) {
   // Parse the pragma directive and get the macro IdentifierInfo*.
   IdentifierInfo *IdentInfo = ParsePragmaPushOrPopMacro(PopMacroTok);
   if (!IdentInfo) return;
 
   // Find the vector<MacroInfo*> associated with the macro.
   llvm::DenseMap<IdentifierInfo*, std::vector<MacroInfo*> >::iterator iter =
     PragmaPushMacroInfo.find(IdentInfo);
   if (iter != PragmaPushMacroInfo.end()) {
-    // Release the MacroInfo currently associated with IdentInfo.
-    MacroInfo *CurrentMI = getMacroInfo(IdentInfo);
-    if (CurrentMI) {
+    // Forget the MacroInfo currently associated with IdentInfo.
+    if (MacroInfo *CurrentMI = getMacroInfo(IdentInfo)) {
       if (CurrentMI->isWarnIfUnused())
         WarnUnusedMacroLocs.erase(CurrentMI->getDefinitionLoc());
-      ReleaseMacroInfo(CurrentMI);
     }
 
     // Get the MacroInfo we want to reinstall.
     MacroInfo *MacroToReInstall = iter->second.back();
 
     // Reinstall the previously pushed macro.
-    setMacroInfo(IdentInfo, MacroToReInstall);
+    setMacroInfo(IdentInfo, MacroToReInstall, MessageLoc);
 
     // Pop PragmaPushMacroInfo stack.
     iter->second.pop_back();
     if (iter->second.size() == 0)
       PragmaPushMacroInfo.erase(iter);
   } else {
     Diag(MessageLoc, diag::warn_pragma_pop_macro_no_push)
       << IdentInfo->getName();

tools/clang/lib/Sema/SemaCodeComplete.cpp

static void AddMacroResults(Preprocessor &PP, ResultBuilder &Results,
                             bool TargetTypeIsPointer = false) {
   typedef CodeCompletionResult Result;
   
   Results.EnterNewScope();
   
   for (Preprocessor::macro_iterator M = PP.macro_begin(), 
                                  MEnd = PP.macro_end();
        M != MEnd; ++M) {
+    if (M->second->MI)
       Results.AddResult(Result(M->first, 
                              getMacroUsagePriority(M->first->getName(),
                                                    PP.getLangOpts(),
                                                    TargetTypeIsPointer)));
   }
   
   Results.ExitScope();
   
 }
void Sema::CodeCompletePreprocessorMacroName(bool IsDefinition) {
   if (!IsDefinition && (!CodeCompleter || CodeCompleter->includeMacros())) {
     // Add just the names of macros, not their arguments.    
     CodeCompletionBuilder Builder(Results.getAllocator(),
                                   Results.getCodeCompletionTUInfo());
     Results.EnterNewScope();
     for (Preprocessor::macro_iterator M = PP.macro_begin(), 
                                    MEnd = PP.macro_end();
          M != MEnd; ++M) {
+      if (M->second->MI) {
         Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(
                                            M->first->getName()));
         Results.AddResult(Builder.TakeString());
       }
+    }
     Results.ExitScope();
   } else if (IsDefinition) {
     // FIXME: Can we detect when the user just wrote an include guard above?
   }
   
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size()); 
 }

tools/clang/lib/Serialization/ASTReader.cpp

case PP_MACRO_FUNCTION_LIKE: {
         MI->setIsFunctionLike();
         if (isC99VarArgs) MI->setIsC99Varargs();
         if (isGNUVarArgs) MI->setIsGNUVarargs();
         MI->setArgumentList(MacroArgs.data(), MacroArgs.size(),
                             PP.getPreprocessorAllocator());
       }
 
       // Finally, install the macro.
-      PP.setMacroInfo(II, MI, /*LoadedFromAST=*/true);
+      PP.setMacroInfo(II, MI, Loc, /*LoadedFromAST=*/true);
 
       // Remember that we saw this macro last so that we add the tokens that
       // form its body to it.
       Macro = MI;
 
       if (NextIndex + 1 == Record.size() && PP.getPreprocessingRecord() &&
           Record[NextIndex]) {
         // We have a macro definition. Register the association

tools/clang/lib/Serialization/ASTWriter.cpp

void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
 
   // Construct the list of macro definitions that need to be serialized.
   SmallVector<std::pair<const IdentifierInfo *, MacroInfo *>, 2> 
     MacrosToEmit;
   llvm::SmallPtrSet<const IdentifierInfo*, 4> MacroDefinitionsSeen;
   for (Preprocessor::macro_iterator I = PP.macro_begin(Chain == 0), 
                                     E = PP.macro_end(Chain == 0);
        I != E; ++I) {
-    const IdentifierInfo *Name = I->first;
-    if (!IsModule || I->second->isPublic()) {
-      MacroDefinitionsSeen.insert(Name);
-      MacrosToEmit.push_back(std::make_pair(I->first, I->second));
+    // We don't store macro history, as we don't currently needed it for PCH.
+    if (I->second->MI) {
+      if (!IsModule || I->second->MI->isPublic()) {
+        MacroDefinitionsSeen.insert(I->first);
+        MacrosToEmit.push_back(std::make_pair(I->first, I->second->MI));
+      }
     }
   }
   
   // Sort the set of macro definitions that need to be serialized by the
   // name of the macro, to provide a stable ordering.
   llvm::array_pod_sort(MacrosToEmit.begin(), MacrosToEmit.end(), 
                        &compareMacroDefinitions);