rsmith accepted this revision.
rsmith added a comment.
Some comments, but I'm happy for you to go ahead and commit after addressing
them. Thanks!
================
Comment at: include/clang/Lex/Preprocessor.h:2004
+ ArrayRef<PPConditionalInfo> getPreambleConditionalStack() const
+ { return PreambleConditionalStack.getStack(); }
+
----------------
Please put the `{` on the previous line and the `}` on the next line. We don't
use this "sandwich braces" style in clang except when defining the function on
the same line as the declaration.
================
Comment at: include/clang/Lex/PreprocessorLexer.h:182
+ void setConditionalLevels(ArrayRef<PPConditionalInfo> CL)
+ {
+ ConditionalStack.clear();
----------------
`{` on the previous line, please.
================
Comment at: lib/Lex/Lexer.cpp:2548
+ if (PP->isRecordingPreamble()) {
+ PP->setRecordedPreambleConditionalStack(ConditionalStack);
+ if (PP->isInPreamble())
----------------
This should be in the `isInPreamble()` conditional below, too. We don't want to
make a redundant copy of the `ConditionalStack` at the end of each `#include`d
file, just at the end of the overall preamble.
================
Comment at: lib/Lex/PPLexerChange.cpp:49
+bool Preprocessor::isInPreamble() const {
+ if (IsFileLexer())
----------------
I think this would be better named as `isInMainFile`: we don't care whether
we're in the preamble, or even if there is one, here (the caller checks that).
https://reviews.llvm.org/D15994
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
