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

Richard Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 7 14:04:54 PST 2017


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





More information about the cfe-commits mailing list