[cfe-commits] [PATCH] Keep history of macro definitions and #undefs

[Doug Gregor]

  Thanks for working on this! Some comments inline.


================
Comment at: tools/clang/include/clang/Lex/Preprocessor.h:476
@@ +475,3 @@
+    /// SourceLocation for built-in macros.
+    SourceLocation DefinitionLoc;
+    /// Next node in the list.
----------------
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.

================
Comment at: tools/clang/include/clang/Lex/Preprocessor.h:478
@@ +477,3 @@
+    /// Next node in the list.
+    MacroInfoList *Next;
+
----------------
This is really a link to the previously-defined macro, right?

================
Comment at: tools/clang/lib/Lex/PPMacroExpansion.cpp:62
@@ -62,1 +61,3 @@
+    // This case is handled in the caller
+    assert(0 && "Trying to undefine undefined macro!");
   }
----------------
llvm_unreachable?

================
Comment at: tools/clang/lib/Sema/SemaCodeComplete.cpp:2907
@@ -2906,2 +2906,3 @@
        M != MEnd; ++M) {
-    Results.AddResult(Result(M->first, 
+    if (M->second->MI)
+      Results.AddResult(Result(M->first, 
----------------
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.

================
Comment at: tools/clang/lib/Sema/SemaCodeComplete.cpp:7174
@@ -7172,2 +7173,3 @@
          M != MEnd; ++M) {
-      Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(
+      if (M->second->MI) {
+        Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(
----------------
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.

================
Comment at: tools/clang/lib/Serialization/ASTWriter.cpp:1679
@@ -1678,5 +1678,3 @@
        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) {
----------------
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.


http://llvm-reviews.chandlerc.com/D28