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

[Alexander Kornienko]

  Sending the new patch right after this comment.


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

================
Comment at: tools/clang/include/clang/Lex/Preprocessor.h:478
@@ +477,3 @@
+    /// Next node in the list.
+    MacroInfoList *Next;
+
----------------
Doug Gregor wrote:
> This is really a link to the previously-defined macro, right?
It's 'Next' in terms of linked list, but I could make it 'Previous' if you like ;)

================
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) {
----------------
Doug Gregor wrote:
> 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.
Added FIXME.

================
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!");
   }
----------------
Doug Gregor wrote:
> llvm_unreachable?
Yep. Done.

================
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, 
----------------
Doug Gregor wrote:
> 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.
Done.

================
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(
----------------
Doug Gregor wrote:
> 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.
Reverted this.


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