[cfe-commits] [PATCH] Macro history (de-)serialization.

Douglas Gregor dgregor at apple.com
Thu Sep 13 22:47:35 PDT 2012


(Second try for Phab's sake)


On Sep 12, 2012, at 1:58 PM, Alexander Kornienko <reviews at llvm-reviews.chandlerc.com> wrote:

> Hi doug.gregor,
> 
> Passes all tests (+ the new one with code completion), but needs a thorough review in part related to modules.
> 
> http://llvm-reviews.chandlerc.com/D41
> 
> Files:
>  tools/clang/test/CodeCompletion/Inputs/macros.h
>  tools/clang/test/CodeCompletion/macros.c
>  tools/clang/include/clang/Basic/IdentifierTable.h
>  tools/clang/include/clang/Lex/Preprocessor.h
>  tools/clang/lib/Basic/IdentifierTable.cpp
>  tools/clang/lib/Lex/PPMacroExpansion.cpp
>  tools/clang/lib/Serialization/ASTWriter.cpp
>  tools/clang/lib/Serialization/ASTReader.cpp
> <D41.1.patch>

+    // History of macro definitions for this identifier in chronological order.
+    SmallVector<MacroInfo*, 8> MacroHistory;
+    while (MI) {
+      MacroHistory.push_back(MI);
+      MI = MI->getPreviousDefinition();
+    }
+
+    while (!MacroHistory.empty()) {
+      MI = MacroHistory.pop_back_val();

While this will write out all of the macro definitions and undefs, it has a few issues:

 - There's no way for the reader to know that more than one macro definition is coming, and in fact the reader looks like it will only end up reading the *first* macro definition, rather than the *last* one. I suspect that #define'ing then #undef'ing a macro and defining it again within a PCH file will lead to the wrong macro definition in a client of that PCH file
 - It forces the reader to load in all of the macro history to get to the most recent one, which will be wasted effort in most cases: the vast majority of clients only care about the most recent macro definition and the rest of the definitions could be loaded lazily (e.g., the way we lazily load the body of a function).
 - This always writes out all of the macro history, even in chained PCH, where some of the macro history might already be in the original PCH. (This kind of issue matters more for modules).

	- Doug





More information about the cfe-commits mailing list