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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
