hans added a comment.
This looks reasonable to me as far as I can tell. Thanks!
I think the expert on the driver side for this stuff is Nico, so hopefully he
can take a look as well.
================
Comment at: include/clang/Basic/DiagnosticLexKinds.td:412
+ "%select{create|use}1 precompiled header">, DefaultFatal;
+def err_pp_macro_def_mismatch_with_pch : Warning<
+ "definition of macro %0 does not match definition in precompiled header">,
----------------
Should probably just have a warn_ prefix.
================
Comment at: include/clang/Driver/CC1Options.td:604
+ HelpText<"When creating a pch stop at this file. When using a pch start "
+ "after this file.">;
def fno_pch_timestamp : Flag<["-"], "fno-pch-timestamp">,
----------------
The "through header" terminology was new to me, and I didn't see it when
browsing the MSDN articles about precompiled headers. The HelpText here
probably isn't the right place, but it would be good if the term could be
documented somewhere to make it clearer exactly what the behaviour is. It's not
really obvious to me what "stop at this file" and "start after this file"
means. I can guess, but it would be nice if it were more explicit :-)
================
Comment at: include/clang/Lex/Preprocessor.h:2050
+ PCHThroughHeaderFileID = FID;
+ }
+
----------------
I would probably define this out-of-line like the other methods you added, but
no big deal.
================
Comment at: lib/Driver/ToolChains/Clang.cpp:1091
+ Args.MakeArgString(Twine("-pch-through-header=") + ThroughHeader));
}
}
----------------
In r335466, I added the -building-pch-with-obj flag. This is probably the right
place to pass it when there's a /Yc flag.
================
Comment at: lib/Lex/PPDirectives.cpp:1887
+ SkippingUntilPCHThroughHeader = false;
+ return;
+ }
----------------
Returning here seemed surprising to me. Isn't just setting the flag and then
carrying on as usual what we want?
================
Comment at: lib/Lex/PPLexerChange.cpp:344
const bool LeavingSubmodule = CurLexer && CurLexerSubmodule;
+ bool LeavingPCHThroughHeader = false;
if ((LeavingSubmodule || IncludeMacroStack.empty()) &&
----------------
Maybe move this down to where it's first used?
================
Comment at: lib/Lex/Preprocessor.cpp:583
+
+/// Return true if the FileEntry is the PCH through header.
+bool Preprocessor::isPCHThroughHeader(const FileEntry *FE) {
----------------
I know some functions already do this, but I don't think repeating the comment
from the .h file here is necessary, it's just another thing to keep in sync.
Same for the other small methods below.
================
Comment at: lib/Lex/Preprocessor.cpp:607
+void Preprocessor::SkipTokensUntilPCHThroughHeader()
+{
+ bool ReachedMainFileEOF = false;
----------------
nit: curly on the previous line
================
Comment at: test/PCH/pch-through2.cpp:3
+// RUN: %clang_cc1 -I %S -emit-pch \
+// RUN: -pch-through-header=Inputs/pch-through2.h -o %t.s2t2 %s
+
----------------
What's s2t2 for? From what I see, there's only two pch files used in this test,
so I would have gone with %t.1 and %t.2
https://reviews.llvm.org/D46652
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
