rsmith added inline comments.
================
Comment at: include/clang/Lex/Preprocessor.h:310
+
+ const Stack &getStack() const {
+ assert(ConditionalStack);
----------------
Return an `ArrayRef` rather than exposing the type of the storage (which is an
implementation detail), here and transitively through the callers of this.
================
Comment at: include/clang/Lex/Preprocessor.h:322
+
+ void setStack(const Stack &s) {
+ if (!isRecording() && !isReplaying())
----------------
Please pass in an `ArrayRef` here rather than a `SmallVector`.
================
Comment at: include/clang/Lex/Preprocessor.h:328
+ else
+ ConditionalStack = new Stack(s);
+ }
----------------
I don't see a need to heap-allocate this separately from the `Preprocessor`.
================
Comment at: include/clang/Lex/PreprocessorOptions.h:84
+
+ bool PreambleGeneration = false;
----------------
Please add a doc comment for this option. Also, maybe `GeneratePreamble` to
avoid ambiguity as to whether this is some kind of generation number for the
preamble?
================
Comment at: lib/Lex/PPLexerChange.cpp:139-142
+ if (PreambleConditionalStack.isReplaying()) {
+ CurPPLexer->setConditionalLevels(PreambleConditionalStack.getStack());
+ PreambleConditionalStack.doneReplaying();
+ }
----------------
I think this would make more sense two callers up, in
`Preprocessor::EnterMainSourceFile`
https://reviews.llvm.org/D15994
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
