LineIterator: Provide a variant that keeps blank lines

It isn't always useful to skip blank lines, as evidenced by the
somewhat awkward use of line_iterator in llvm-cov. This adds a knob to
control whether or not to skip blanks.

llvm-svn: 217960

llvm/include/llvm/ProfileData/InstrProfReader.h

@@ -120,7 +120,7 @@ private:
     LLVM_DELETED_FUNCTION;
 public:
   TextInstrProfReader(std::unique_ptr<MemoryBuffer> DataBuffer_)
-      : DataBuffer(std::move(DataBuffer_)), Line(*DataBuffer, '#') {}
+      : DataBuffer(std::move(DataBuffer_)), Line(*DataBuffer, true, '#') {}
 
   /// Read the header.
   std::error_code readHeader() override { return success(); }

llvm/include/llvm/Support/LineIterator.h

@@ -18,20 +18,22 @@ namespace llvm {
 
 class MemoryBuffer;
 
-/// \brief A forward iterator which reads non-blank text lines from a buffer.
+/// \brief A forward iterator which reads text lines from a buffer.
 ///
 /// This class provides a forward iterator interface for reading one line at
 /// a time from a buffer. When default constructed the iterator will be the
 /// "end" iterator.
 ///
-/// The iterator also is aware of what line number it is currently processing
+/// The iterator is aware of what line number it is currently processing. It
-/// and can strip comment lines given the comment-starting character.
+/// strips blank lines by default, and comment lines given a comment-starting
+/// character.
 ///
 /// Note that this iterator requires the buffer to be nul terminated.
 class line_iterator
     : public std::iterator<std::forward_iterator_tag, StringRef> {
   const MemoryBuffer *Buffer;
   char CommentMarker;
+  bool SkipBlanks;
 
   unsigned LineNumber;
   StringRef CurrentLine;
@@ -41,7 +43,8 @@ public:
   line_iterator() : Buffer(nullptr) {}
 
   /// \brief Construct a new iterator around some memory buffer.
-  explicit line_iterator(const MemoryBuffer &Buffer, char CommentMarker = '\0');
+  explicit line_iterator(const MemoryBuffer &Buffer, bool SkipBlanks = true,
+                         char CommentMarker = '\0');
 
   /// \brief Return true if we've reached EOF or are an "end" iterator.
   bool is_at_eof() const { return !Buffer; }

llvm/lib/ProfileData/SampleProfReader.cpp

@@ -159,7 +159,7 @@ bool SampleProfileReader::loadText() {
     return false;
   }
   MemoryBuffer &Buffer = *BufferOrErr.get();
-  line_iterator LineIt(Buffer, '#');
+  line_iterator LineIt(Buffer, /*SkipBlanks=*/true, '#');
 
   // Read the profile of each function. Since each function may be
   // mentioned more than once, and we are collecting flat profiles,

llvm/lib/Support/LineIterator.cpp

@@ -12,16 +12,19 @@
 
 using namespace llvm;
 
-line_iterator::line_iterator(const MemoryBuffer &Buffer, char CommentMarker)
+line_iterator::line_iterator(const MemoryBuffer &Buffer, bool SkipBlanks,
+                             char CommentMarker)
     : Buffer(Buffer.getBufferSize() ? &Buffer : nullptr),
-      CommentMarker(CommentMarker), LineNumber(1),
+      CommentMarker(CommentMarker), SkipBlanks(SkipBlanks), LineNumber(1),
       CurrentLine(Buffer.getBufferSize() ? Buffer.getBufferStart() : nullptr,
                   0) {
   // Ensure that if we are constructed on a non-empty memory buffer that it is
   // a null terminated buffer.
   if (Buffer.getBufferSize()) {
     assert(Buffer.getBufferEnd()[0] == '\0');
-    advance();
+    // Make sure we don't skip a leading newline if we're keeping blanks
+    if (SkipBlanks || *Buffer.getBufferStart() != '\n')
+      advance();
   }
 }
 
@@ -31,7 +34,13 @@ void line_iterator::advance() {
   const char *Pos = CurrentLine.end();
   assert(Pos == Buffer->getBufferStart() || *Pos == '\n' || *Pos == '\0');
 
-  if (CommentMarker == '\0') {
+  if (*Pos == '\n') {
+    ++Pos;
+    ++LineNumber;
+  }
+  if (!SkipBlanks && *Pos == '\n') {
+    // Nothing to do for a blank line.
+  } else if (CommentMarker == '\0') {
     // If we're not stripping comments, this is simpler.
     size_t Blanks = 0;
     while (Pos[Blanks] == '\n')
@@ -41,6 +50,8 @@ void line_iterator::advance() {
   } else {
     // Skip comments and count line numbers, which is a bit more complex.
     for (;;) {
+      if (*Pos == '\n' && !SkipBlanks)
+        break;
       if (*Pos == CommentMarker)
         do {
           ++Pos;
@@ -61,9 +72,9 @@ void line_iterator::advance() {
 
   // Measure the line.
   size_t Length = 0;
-  do {
+  while (Pos[Length] != '\0' && Pos[Length] != '\n') {
     ++Length;
-  } while (Pos[Length] != '\0' && Pos[Length] != '\n');
+  }
 
   CurrentLine = StringRef(Pos, Length);
 }

llvm/tools/llvm-cov/SourceCoverageView.cpp

@@ -206,7 +206,7 @@ void SourceCoverageView::render(raw_ostream &OS, unsigned IndentLevel) {
   size_t CurrentHighlightRange = 0;
   size_t CurrentRegionMarker = 0;
 
-  line_iterator Lines(File);
+  line_iterator Lines(File, /*SkipBlanks=*/false);
   // Advance the line iterator to the first line.
   while (Lines.line_number() < LineOffset)
     ++Lines;
@@ -228,8 +228,9 @@ void SourceCoverageView::render(raw_ostream &OS, unsigned IndentLevel) {
   auto NextISV = InstantiationSubViews.begin();
   auto EndISV = InstantiationSubViews.end();
 
-  for (size_t I = 0, E = LineStats.size(); I < E; ++I) {
+  for (size_t I = 0, E = LineStats.size(); I < E && !Lines.is_at_eof();
-    unsigned LineNo = I + LineOffset;
+       ++I, ++Lines) {
+    unsigned LineNo = Lines.line_number();
 
     renderIndent(OS, IndentLevel);
     if (Options.ShowLineStats)
@@ -252,11 +253,6 @@ void SourceCoverageView::render(raw_ostream &OS, unsigned IndentLevel) {
 
     // Display the source code for the current line.
     StringRef Line = *Lines;
-    // Check if the line is empty, as line_iterator skips blank lines.
-    if (LineNo < Lines.line_number())
-      Line = "";
-    else if (!Lines.is_at_eof())
-      ++Lines;
     renderLine(OS, Line, LineRanges);
 
     // Show the region markers.

llvm/unittests/Support/LineIteratorTest.cpp

@@ -40,15 +40,17 @@ TEST(LineIteratorTest, Basic) {
   EXPECT_EQ(E, I);
 }
 
-TEST(LineIteratorTest, CommentSkipping) {
+TEST(LineIteratorTest, CommentAndBlankSkipping) {
   std::unique_ptr<MemoryBuffer> Buffer(
       MemoryBuffer::getMemBuffer("line 1\n"
                                  "line 2\n"
                                  "# Comment 1\n"
-                                 "line 4\n"
+                                 "\n"
+                                 "line 5\n"
+                                 "\n"
                                  "# Comment 2"));
 
-  line_iterator I = line_iterator(*Buffer, '#'), E;
+  line_iterator I = line_iterator(*Buffer, true, '#'), E;
 
   EXPECT_FALSE(I.is_at_eof());
   EXPECT_NE(E, I);
@@ -59,14 +61,51 @@ TEST(LineIteratorTest, CommentSkipping) {
   EXPECT_EQ("line 2", *I);
   EXPECT_EQ(2, I.line_number());
   ++I;
-  EXPECT_EQ("line 4", *I);
+  EXPECT_EQ("line 5", *I);
-  EXPECT_EQ(4, I.line_number());
+  EXPECT_EQ(5, I.line_number());
   ++I;
 
   EXPECT_TRUE(I.is_at_eof());
   EXPECT_EQ(E, I);
 }
 
+TEST(LineIteratorTest, CommentSkippingKeepBlanks) {
+  std::unique_ptr<MemoryBuffer> Buffer(
+      MemoryBuffer::getMemBuffer("line 1\n"
+                                 "line 2\n"
+                                 "# Comment 1\n"
+                                 "# Comment 2\n"
+                                 "\n"
+                                 "line 6\n"
+                                 "\n"
+                                 "# Comment 3"));
+
+  line_iterator I = line_iterator(*Buffer, false, '#'), E;
+
+  EXPECT_FALSE(I.is_at_eof());
+  EXPECT_NE(E, I);
+
+  EXPECT_EQ("line 1", *I);
+  EXPECT_EQ(1, I.line_number());
+  ++I;
+  EXPECT_EQ("line 2", *I);
+  EXPECT_EQ(2, I.line_number());
+  ++I;
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(5, I.line_number());
+  ++I;
+  EXPECT_EQ("line 6", *I);
+  EXPECT_EQ(6, I.line_number());
+  ++I;
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(7, I.line_number());
+  ++I;
+
+  EXPECT_TRUE(I.is_at_eof());
+  EXPECT_EQ(E, I);
+}
+
+
 TEST(LineIteratorTest, BlankSkipping) {
   std::unique_ptr<MemoryBuffer> Buffer = MemoryBuffer::getMemBuffer("\n\n\n"
                                                                     "line 1\n"
@@ -90,10 +129,49 @@ TEST(LineIteratorTest, BlankSkipping) {
   EXPECT_EQ(E, I);
 }
 
+TEST(LineIteratorTest, BlankKeeping) {
+  std::unique_ptr<MemoryBuffer> Buffer = MemoryBuffer::getMemBuffer("\n\n"
+                                                                    "line 3\n"
+                                                                    "\n"
+                                                                    "line 5\n"
+                                                                    "\n\n");
+  line_iterator I = line_iterator(*Buffer, false), E;
+
+  EXPECT_FALSE(I.is_at_eof());
+  EXPECT_NE(E, I);
+
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(1, I.line_number());
+  ++I;
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(2, I.line_number());
+  ++I;
+  EXPECT_EQ("line 3", *I);
+  EXPECT_EQ(3, I.line_number());
+  ++I;
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(4, I.line_number());
+  ++I;
+  EXPECT_EQ("line 5", *I);
+  EXPECT_EQ(5, I.line_number());
+  ++I;
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(6, I.line_number());
+  ++I;
+  EXPECT_EQ("", *I);
+  EXPECT_EQ(7, I.line_number());
+  ++I;
+
+  EXPECT_TRUE(I.is_at_eof());
+  EXPECT_EQ(E, I);
+}
+
 TEST(LineIteratorTest, EmptyBuffers) {
   std::unique_ptr<MemoryBuffer> Buffer = MemoryBuffer::getMemBuffer("");
   EXPECT_TRUE(line_iterator(*Buffer).is_at_eof());
   EXPECT_EQ(line_iterator(), line_iterator(*Buffer));
+  EXPECT_TRUE(line_iterator(*Buffer, false).is_at_eof());
+  EXPECT_EQ(line_iterator(), line_iterator(*Buffer, false));
 
   Buffer = MemoryBuffer::getMemBuffer("\n\n\n");
   EXPECT_TRUE(line_iterator(*Buffer).is_at_eof());
@@ -102,14 +180,14 @@ TEST(LineIteratorTest, EmptyBuffers) {
   Buffer = MemoryBuffer::getMemBuffer("# foo\n"
                                       "\n"
                                       "# bar");
-  EXPECT_TRUE(line_iterator(*Buffer, '#').is_at_eof());
+  EXPECT_TRUE(line_iterator(*Buffer, true, '#').is_at_eof());
-  EXPECT_EQ(line_iterator(), line_iterator(*Buffer, '#'));
+  EXPECT_EQ(line_iterator(), line_iterator(*Buffer, true, '#'));
 
   Buffer = MemoryBuffer::getMemBuffer("\n"
                                       "# baz\n"
                                       "\n");
-  EXPECT_TRUE(line_iterator(*Buffer, '#').is_at_eof());
+  EXPECT_TRUE(line_iterator(*Buffer, true, '#').is_at_eof());
-  EXPECT_EQ(line_iterator(), line_iterator(*Buffer, '#'));
+  EXPECT_EQ(line_iterator(), line_iterator(*Buffer, true, '#'));
 }
 
 } // anonymous namespace