[PATCH] D15994: Allow for unfinished #if blocks in preambles.

[Richard Smith via Phabricator via cfe-commits]

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