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

481

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.

483

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

tools/clang/lib/Lex/PPMacroExpansion.cpp

54

llvm_unreachable?

tools/clang/lib/Sema/SemaCodeComplete.cpp

2907–2911

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.

7177

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

1680–1685

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

481

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?

483

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

tools/clang/lib/Lex/PPMacroExpansion.cpp

54

Yep. Done.

tools/clang/lib/Sema/SemaCodeComplete.cpp

2907–2911

Done.

7177

Reverted this.

tools/clang/lib/Serialization/ASTWriter.cpp

1680–1685

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/MacroInfo.h

 class MacroInfo {
   //===--------------------------------------------------------------------===//
   // State set when the macro is defined.
 
   /// Location - This is the place the macro is defined.
   SourceLocation Location;
   /// EndLocation - The location of the last token in the macro.
   SourceLocation EndLocation;
+  /// \brief The location where the macro was #undef'd, or an invalid location
+  /// for macros that haven't been undefined.
+  SourceLocation UndefLocation;
+  /// \brief Previous definition, the identifier of this macro was defined to,
+  /// or NULL.
+  MacroInfo *PreviousDefinition;
 
   /// Arguments - The list of arguments for a function-like macro.  This can be
   /// empty, for, e.g. "#define X()".  In a C99-style variadic macro, this
   /// includes the \c __VA_ARGS__ identifier on the list.
   IdentifierInfo **ArgumentList;
   unsigned NumArguments;
 
   /// \brief The location at which this macro was either explicitly exported
public:
 
   /// getDefinitionLoc - Return the location that the macro was defined at.
   ///
   SourceLocation getDefinitionLoc() const { return Location; }
 
   /// setDefinitionEndLoc - Set the location of the last token in the macro.
   ///
   void setDefinitionEndLoc(SourceLocation EndLoc) { EndLocation = EndLoc; }
+
   /// getDefinitionEndLoc - Return the location of the last token in the macro.
   ///
   SourceLocation getDefinitionEndLoc() const { return EndLocation; }
-  
+  /// \brief Set the location where macro was undefined. Can only be set once.
+  void setUndefLoc(SourceLocation UndefLoc) {
+    assert(UndefLocation.isInvalid() && "UndefLocation is already set!");
+    assert(UndefLoc.isValid() && "Invalid UndefLoc!");
+    UndefLocation = UndefLoc;
+  }
+
+  /// \brief Get the location where macro was undefined.
+  SourceLocation getUndefLoc() const { return UndefLocation; }
+
+  /// \brief Set previous definition of the macro with the same name. Can only
+  /// be set once.
+  void setPreviousDefinition(MacroInfo *PreviousDef) {
+    assert(!PreviousDefinition && "PreviousDefiniton is already set!");
+    PreviousDefinition = PreviousDef;
+  }
+
+  /// \brief Get previous definition of the macro with the same name.
+  MacroInfo *getPreviousDefinition() { return PreviousDefinition; }
+
   /// \brief Get length in characters of the macro definition.
   unsigned getDefinitionLength(SourceManager &SM) const {
     if (IsDefinitionLengthCached)
       return DefinitionLength;
     return getDefinitionLengthSlow(SM);
   }
 
   /// isIdenticalTo - Return true if the specified macro definition is equal to

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.
+  /// 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*, 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).
MacroInfo *getMacroInfo(IdentifierInfo *II) const {
 
     return getInfoForMacro(II);
   }
 
   /// \brief Specify a macro for this identifier.
   void setMacroInfo(IdentifierInfo *II, MacroInfo *MI,
                     bool LoadedFromAST = false);
 
-  /// macro_iterator/macro_begin/macro_end - This allows you to walk the current
-  /// state of the macro table.  This visits every currently-defined macro.
+  /// macro_iterator/macro_begin/macro_end - This allows you to walk the macro
+  /// history table. Currently defined macros have
+  /// IdentifierInfo::hasMacroDefinition() set and an empty
+  /// MacroInfo::getUndefLoc() at the head of the list.
   typedef llvm::DenseMap<IdentifierInfo*,
                          MacroInfo*>::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; }
 
   /// getIdentifierInfo - Return information about the specified preprocessor
   /// identifier token.  The version of this method that takes two character
   /// pointers is preferred unless the identifier is already available as a

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->first->hasMacroDefinition())
+      MacrosByID.push_back(id_macro_pair(It->first, It->second));
+  }
   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/MacroInfo.cpp

 // This file implements the MacroInfo interface.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 using namespace clang;
 
-MacroInfo::MacroInfo(SourceLocation DefLoc) : Location(DefLoc) {
-  IsFunctionLike = false;
-  IsC99Varargs = false;
-  IsGNUVarargs = false;
-  IsBuiltinMacro = false;
-  IsFromAST = false;
-  ChangedAfterLoad = false;
-  IsDisabled = false;
-  IsUsed = false;
-  IsAllowRedefinitionsWithoutWarning = false;
-  IsWarnIfUnused = false;
-  IsDefinitionLengthCached = false;
-  IsPublic = true;
-  
-  ArgumentList = 0;
-  NumArguments = 0;
+MacroInfo::MacroInfo(SourceLocation DefLoc)
+  : Location(DefLoc),
+    PreviousDefinition(0),
+    ArgumentList(0),
+    NumArguments(0),
+    IsDefinitionLengthCached(false),
+    IsFunctionLike(false),
+    IsC99Varargs(false),
+    IsGNUVarargs(false),
+    IsBuiltinMacro(false),
+    IsFromAST(false),
+    ChangedAfterLoad(false),
+    IsDisabled(false),
+    IsUsed(false),
+    IsAllowRedefinitionsWithoutWarning(false),
+    IsWarnIfUnused(false),
+    IsPublic(true) {
 }
 
 MacroInfo::MacroInfo(const MacroInfo &MI, llvm::BumpPtrAllocator &PPAllocator) {
   Location = MI.Location;
   EndLocation = MI.EndLocation;
+  UndefLocation = MI.UndefLocation;
+  PreviousDefinition = MI.PreviousDefinition;
   ReplacementTokens = MI.ReplacementTokens;
   IsFunctionLike = MI.IsFunctionLike;
   IsC99Varargs = MI.IsC99Varargs;
   IsGNUVarargs = MI.IsGNUVarargs;
   IsBuiltinMacro = MI.IsBuiltinMacro;
   IsFromAST = MI.IsFromAST;
   ChangedAfterLoad = MI.ChangedAfterLoad;
   IsDisabled = MI.IsDisabled;

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);
 
   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.
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);
+  MI->setUndefLoc(MacroNameTok.getLocation());
+  IdentifierInfo *II = MacroNameTok.getIdentifierInfo();
+  II->setHasMacroDefinition(false);
+  if (II->isFromAST())
+    II->setChangedSinceDeserialization();
 }
 
 
 //===----------------------------------------------------------------------===//
 // 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!");
+  assert(Pos->second->getUndefLoc().isInvalid() && "Macro is undefined!");
   return Pos->second;
 }
 
 /// 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);
-    if (II->isFromAST() && !LoadedFromAST)
-      II->setChangedSinceDeserialization();
-  }
+  assert(MI && "MacroInfo should be non-zero!");
+  MI->setPreviousDefinition(Macros[II]);
 }
 
 /// 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);
 

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);
 

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) {
+    // FIXME: 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).
+    if (M->first->hasMacroDefinition())
       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) {
       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?
   }
   

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;
+    // FIXME: We'll need to store macro history in PCH.
+    if (I->first->hasMacroDefinition()) {
       if (!IsModule || I->second->isPublic()) {
-      MacroDefinitionsSeen.insert(Name);
+        MacroDefinitionsSeen.insert(I->first);
         MacrosToEmit.push_back(std::make_pair(I->first, I->second));
       }
     }
+  }
   
   // 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);
   
   // Resolve any identifiers that defined macros at the time they were
   // deserialized, adding them to the list of macros to emit (if appropriate).