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

[Richard Smith via cfe-commits]

rsmith added inline comments.


================
Comment at: include/clang/Lex/Preprocessor.h:283-286
+      Off = 0,
+      Recording = 1,
+      Replaying = 2,
+      Done = 3
----------------
What's the difference between the `Off` and `Done` states? They seem to be the same to me?


================
Comment at: include/clang/Lex/Preprocessor.h:317
+    void setStack(const Stack &s)
+    {
+      if (!isRecording() && !isReplaying())
----------------
`{` on previous line.


================
Comment at: include/clang/Lex/Preprocessor.h:326
+
+    bool HasRecordedPreamble() const { return ConditionalStack.getPointer(); }
+
----------------
Start function names with a lowercase letter.


================
Comment at: include/clang/Lex/Preprocessor.h:329
+  private:
+    llvm::PointerIntPair<Stack *, 2, State> ConditionalStack;
+  } PreambleConditionalStack;
----------------
We don't really care much about the `Preprocessor` object getting a few dozen bytes larger, since there will typically only be one of them.  I don't think the complexity of a dynamically-allocated stack and a `PointerIntPair` is worthwhile here. Just store a `Stack` and a `State` directly as members.


================
Comment at: include/clang/Lex/Preprocessor.h:1959-1963
+  bool IsRecordingPreamble() const {
+    return PreambleConditionalStack.isRecording();
+  }
+
+  bool HasRecordedPreamble() const {
----------------
Start function names with a lowercase letter.


================
Comment at: lib/Lex/Lexer.cpp:622
         StringRef Keyword = TheTok.getRawIdentifier();
         PreambleDirectiveKind PDK
           = llvm::StringSwitch<PreambleDirectiveKind>(Keyword)
----------------
You can remove the special-case handling for `#if`/`#else`/`#endif` here.


================
Comment at: lib/Lex/Lexer.cpp:2529-2532
+  if (PP->IsRecordingPreamble()) {
+    PP->setRecordedPreambleConditionalStack(ConditionalStack);
+    ConditionalStack.clear();
+  }
----------------
We should only do this if we reach the end of the main source file. If we reach the end of a `#include`'d file with a `#if` still open, we should diagnose that.


================
Comment at: lib/Serialization/ASTReader.cpp:2816
+        }
+        PP.setReplayablePreambleConditionalStack(ConditionalStack);
+      }
----------------
Why can't we set the conditional stack on the `PPLexer` directly from here? (Why do we need to store it separately?)


https://reviews.llvm.org/D15994