thakis added inline comments.
================
Comment at: lib/Lex/PPMacroExpansion.cpp:110-112
+ // FIXME: shouldIgnoreMacro() in ASTWriter also stops at macros from the
+ // predefines buffer in module builds. Do we need to splice to those here
+ // too?
----------------
rsmith wrote:
> 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...
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
1. built-ins (Preprocessor() ctor)
2. predefines predefines (clang::InitializePreprocessor in Frontend)
3. commandline predefines (later in InitializePreprocessor; InitOpts.Macros
loop)
4. pch source is attached after those are set (but before parsing starts)
5. macros from PCH deserialized on-demand, usually through Lexer::LexIdentifier
-> Preprocessor::LookUpIdentifierInfo -> ASTReader::get(llvm::StringRef) ->
~Deserializing RAII dtor -> FinishedDeserializing -> finishPendingActions ->
resolvePendingMacro
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)
https://reviews.llvm.org/D27545
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
