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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
