arphaman added a comment.
Thanks for working on this! I have some comments below:
================
Comment at: include/clang/Lex/Preprocessor.h:291
+
+ void setState(State s) { ConditionalStackState = s; }
+
----------------
`setState` is redundant IMHO, just assign to `ConditionalStackState` when you
use `setState` below.
================
Comment at: include/clang/Lex/Preprocessor.h:301
+
+ const ArrayRef<PPConditionalInfo> getStack() const {
+ return ConditionalStack;
----------------
the `const` is redundant for the return type.
================
Comment at: include/clang/Lex/Preprocessor.h:310
+
+ void setStack(const ArrayRef<PPConditionalInfo> &s) {
+ if (!isRecording() && !isReplaying())
----------------
You can pass `ArrayRef` by value.
================
Comment at: include/clang/Lex/Preprocessor.h:1974
+
+ const ArrayRef<PPConditionalInfo> getPreambleConditionalStack() const
+ { return PreambleConditionalStack.getStack(); }
----------------
the `const` is redundant for the return type.
================
Comment at: include/clang/Lex/Preprocessor.h:1978
+ void setRecordedPreambleConditionalStack(
+ const SmallVector<PPConditionalInfo, 4> &s) {
+ PreambleConditionalStack.setStack(s);
----------------
Please use `ArrayRef` instead of `SmallVector` here and in the function below.
================
Comment at: include/clang/Lex/PreprocessorLexer.h:181
+
+ void setConditionalLevels(const ArrayRef<PPConditionalInfo> &CL)
+ {
----------------
`ArrayRef` can be passed by value.
================
Comment at: include/clang/Lex/PreprocessorOptions.h:87
+ /// When the lexer is done, one of the things that need to be preserved is
the
+ /// conditional #if stack, so the ASTWriter/ASTReader can safe/restore it
when
+ /// processing the rest of the file.
----------------
Typo: `save`
================
Comment at: lib/Serialization/ASTReader.cpp:2896
+ for (unsigned Idx = 0, N = Record.size() - 1; Idx < N; /* in loop */) {
+ auto loc = ReadSourceLocation(F, Record, Idx);
+ bool WasSkipping = Record[Idx++];
----------------
`Loc` should be capitalized.
================
Comment at: lib/Serialization/ASTWriter.cpp:2269
+ if (PP.isRecordingPreamble() && PP.hasRecordedPreamble()) {
+ for (const auto &Cond : PP.getPreambleConditionalStack()) {
+ AddSourceLocation(Cond.IfLoc, Record);
----------------
I think you should add an assertion here that verifies that the ASTWriter isn't
creating a module.
https://reviews.llvm.org/D15994
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
