PCH files store the macro history for a given macro, and the whole history list for one identifier is given to the Preprocessor at once via Preprocessor::setLoadedMacroDirective(). This contained an assert that no macro history exists yet for that identifier. That's usually true, but it's not true for builtin macros, which are created in Preprocessor() before flags and pchs are processed. Luckily, ASTWriter stops writing macro history lists at builtins (see shouldIgnoreMacro() in ASTWriter.cpp), so the head of the history list was missing for builtin macros. So make the assert weaker, and splice the history list to the existing single define for builtins.
Details
Diff Detail
Event Timeline
Thanks!
test/PCH/builtin-macro.c | ||
---|---|---|
30 | Oh, you mean to check that the DATE value from -D is used, instead of the value of the builtin? Your suggestion works, good idea. |
lib/Lex/PPMacroExpansion.cpp | ||
---|---|---|
110–112 | If I remember correctly, we shouldn't need to; we run this step before we start lexing the predefines buffer. However, that does mean that macros on the command line will *override* macros from the PCH, which seems like it's probably the wrong behavior... |
update a comment
lib/Lex/PPMacroExpansion.cpp | ||
---|---|---|
110–112 | TL;DR: You're right, updated comment, filed PR31324 and PR31326 for (pre-existing) bugs I found along the way. From what I understand, the order is: FrontendAction::BeginSourceFile() builds PCH reader
Aha, you're saying that when the preamble is parsed, when the preprocessor sees #define __STD_HOSTED__ 1 from the preamble, it'll then read the history for __STD_HOSTED__, call this, and then…override it with the default value from the preamble? Doesn't that means that even the default values override the pch? Hm, no, this seems to work: $ cat foo.h #define __STDC_HOSTED__ "hi" $ cat foo.c const char* s = __STDC_HOSTED__; $ bin/clang -cc1 -emit-pch -o foo.h.pch foo.h setting predefines foo.h:1:9: warning: '__STDC_HOSTED__' macro redefined #define __STDC_HOSTED__ "hi" ^ <built-in>:307:9: note: previous definition is here #define __STDC_HOSTED__ 1 ^ 1 warning generated. $ bin/clang -cc1 -include-pch foo.h.pch foo.c -emit-llvm -o - ; ModuleID = 'foo.c' source_filename = "foo.c" target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-apple-darwin14.5.0" @.str = private unnamed_addr constant [3 x i8] c"hi\00", align 1 …ah, but I tested with a pch, not a module, and with a module __STD_HOSTED__ is probably supposed to expand to the "default" value even if it's defined in some used module. How do I do the same with a module instead of a pch? …apparently something like this: $ cat mod/module.map module foo { header "foo.h" } $ cat mod/foo.h #define __STDC_HOSTED__ "hi" $ cat foo.c const char* s = __STDC_HOSTED__; $ bin/clang -cc1 -fmodules -fmodule-name=foo -emit-module mod/module.map -o a.pcm While building module 'foo': In file included from <module-includes>:1: mod/foo.h:1:9: warning: '__STDC_HOSTED__' macro redefined #define __STDC_HOSTED__ "hi" ^ <built-in>:307:9: note: previous definition is here #define __STDC_HOSTED__ 1 ^ 1 warning generated. $ bin/clang -cc1 -fmodules -fmodule-file=a.pcm -emit-obj foo.c foo.c:1:13: warning: incompatible integer to pointer conversion initializing 'const char *' with an expression of type 'int' const char* s = __STDC_HOSTED__; ^ ~~~~~~~~~~~~~~~ 1 warning generated. So yes, looks like this does get the default value there. (Is there some less wordy thing to compile and use a module on the command line for testing? Do I have to make a module.map file like I did? Requires quite a bit more typing than with a pch…) Aha, this comment in ASTReader::get() explains things: // We don't need to do identifier table lookups in C++ modules (we preload // all interesting declarations, and don't need to use the scope for name // lookups). Perform the lookup in PCH files, though, since we don't build // a complete initial identifier table if we're carrying on from a PCH. So in modules we preload everything and then override from the predefines. I updated the comment. You're right that commandline define flags override values from a pch: $ cat foo.h #define __STDC_HOSTED__ "hi" $ cat foo.c const char* s = __STDC_HOSTED__; $ bin/clang -cc1 -emit-pch -o foo.h.pch foo.h foo.h:1:9: warning: '__STDC_HOSTED__' macro redefined #define __STDC_HOSTED__ "hi" ^ <built-in>:307:9: note: previous definition is here #define __STDC_HOSTED__ 1 ^ 1 warning generated. $ bin/clang -cc1 -D__STDC_HOSTED__=4 -include-pch foo.h.pch -emit-obj -o foo.o foo.c <built-in>:1:9: warning: '__STDC_HOSTED__' macro redefined #define __STDC_HOSTED__ 4 ^ /Users/thakis/src/llvm-build/foo.h:1:9: note: previous definition is here #define __STDC_HOSTED__ "hi" ^ foo.c:1:13: warning: incompatible integer to pointer conversion initializing 'const char *' with an expression of type 'int' const char* s = __STDC_HOSTED__; ^ ~~~~~~~~~~~~~~~ 2 warnings generated. $ bin/clang -cc1 -D__STDC_HOSTED__=4 -include foo.h -emit-obj -o foo.o foo.c In file included from <built-in>:311: <command line>:1:9: warning: '__STDC_HOSTED__' macro redefined #define __STDC_HOSTED__ 4 ^ <built-in>:307:9: note: previous definition is here #define __STDC_HOSTED__ 1 ^ In file included from <built-in>:1: ./foo.h:1:9: warning: '__STDC_HOSTED__' macro redefined #define __STDC_HOSTED__ "hi" ^ <command line>:1:9: note: previous definition is here #define __STDC_HOSTED__ 4 ^ 2 warnings generated. I filed PR31324 for that. (Also found an unrelated crasher, PR31326) |
(The reason that the predefines buffer doesn't override the value of __STDC_HOSTED__ from the pch is that CompilerInstance::createPCHExternalASTSource overrides the predefines buffer with a different predefines buffer suggested by the ASTReader, usually an empty one.)
If I remember correctly, we shouldn't need to; we run this step before we start lexing the predefines buffer. However, that does mean that macros on the command line will *override* macros from the PCH, which seems like it's probably the wrong behavior...