[clang][Diagnostics] Split source ranges into line ranges before...

... emitting them.

This makes later code easier to understand, since we emit the code
snippets line by line anyway.
It also fixes the weird underlinig of multi-line source ranges.

Differential Revision: https://reviews.llvm.org/D151215

diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp
index ad5f1d4..c17508f 100644 (file)
--- a/clang/lib/Frontend/TextDiagnostic.cpp
+++ b/clang/lib/Frontend/TextDiagnostic.cpp
@@ -945,87 +945,43 @@ maybeAddRange(std::pair<unsigned, unsigned> A, std::pair<unsigned, unsigned> B,
   return A;
 }
 
-/// Highlight a SourceRange (with ~'s) for any characters on LineNo.
-static void highlightRange(const CharSourceRange &R,
-                           unsigned LineNo, FileID FID,
-                           const SourceColumnMap &map,
-                           std::string &CaretLine,
-                           const SourceManager &SM,
-                           const LangOptions &LangOpts) {
-  if (!R.isValid()) return;
-
-  SourceLocation Begin = R.getBegin();
-  SourceLocation End = R.getEnd();
-
-  unsigned StartLineNo = SM.getExpansionLineNumber(Begin);
-  if (StartLineNo > LineNo || SM.getFileID(Begin) != FID)
-    return;  // No intersection.
-
-  unsigned EndLineNo = SM.getExpansionLineNumber(End);
-  if (EndLineNo < LineNo || SM.getFileID(End) != FID)
-    return;  // No intersection.
-
-  // Compute the column number of the start.
-  unsigned StartColNo = 0;
-  if (StartLineNo == LineNo) {
-    StartColNo = SM.getExpansionColumnNumber(Begin);
-    if (StartColNo) --StartColNo;  // Zero base the col #.
-  }
-
-  // Compute the column number of the end.
-  unsigned EndColNo = map.getSourceLine().size();
-  if (EndLineNo == LineNo) {
-    EndColNo = SM.getExpansionColumnNumber(End);
-    if (EndColNo) {
-      --EndColNo;  // Zero base the col #.
-
-      // Add in the length of the token, so that we cover multi-char tokens if
-      // this is a token range.
-      if (R.isTokenRange())
-        EndColNo += Lexer::MeasureTokenLength(End, SM, LangOpts);
-    } else {
-      EndColNo = CaretLine.size();
-    }
-  }
-
-  assert(StartColNo <= EndColNo && "Invalid range!");
-
-  // Check that a token range does not highlight only whitespace.
-  if (R.isTokenRange()) {
-    // Pick the first non-whitespace column.
-    while (StartColNo < map.getSourceLine().size() &&
-           (map.getSourceLine()[StartColNo] == ' ' ||
-            map.getSourceLine()[StartColNo] == '\t'))
-      StartColNo = map.startOfNextColumn(StartColNo);
-
-    // Pick the last non-whitespace column.
-    if (EndColNo > map.getSourceLine().size())
-      EndColNo = map.getSourceLine().size();
-    while (EndColNo &&
-           (map.getSourceLine()[EndColNo-1] == ' ' ||
-            map.getSourceLine()[EndColNo-1] == '\t'))
-      EndColNo = map.startOfPreviousColumn(EndColNo);
-
-    // If the start/end passed each other, then we are trying to highlight a
-    // range that just exists in whitespace. That most likely means we have
-    // a multi-line highlighting range that covers a blank line.
-    if (StartColNo > EndColNo) {
-      assert(StartLineNo != EndLineNo && "trying to highlight whitespace");
-      StartColNo = EndColNo;
-    }
-  }
+struct LineRange {
+  unsigned LineNo;
+  unsigned StartCol;
+  unsigned EndCol;
+};
 
-  assert(StartColNo <= map.getSourceLine().size() && "Invalid range!");
-  assert(EndColNo <= map.getSourceLine().size() && "Invalid range!");
+/// Highlight \p R (with ~'s) on the current source line.
+static void highlightRange(const LineRange &R, const SourceColumnMap &Map,
+                           std::string &CaretLine) {
+  // Pick the first non-whitespace column.
+  unsigned StartColNo = R.StartCol;
+  while (StartColNo < Map.getSourceLine().size() &&
+         (Map.getSourceLine()[StartColNo] == ' ' ||
+          Map.getSourceLine()[StartColNo] == '\t'))
+    StartColNo = Map.startOfNextColumn(StartColNo);
+
+  // Pick the last non-whitespace column.
+  unsigned EndColNo =
+      std::min(static_cast<size_t>(R.EndCol), Map.getSourceLine().size());
+  while (EndColNo && (Map.getSourceLine()[EndColNo - 1] == ' ' ||
+                      Map.getSourceLine()[EndColNo - 1] == '\t'))
+    EndColNo = Map.startOfPreviousColumn(EndColNo);
+
+  // If the start/end passed each other, then we are trying to highlight a
+  // range that just exists in whitespace. That most likely means we have
+  // a multi-line highlighting range that covers a blank line.
+  if (StartColNo > EndColNo)
+    return;
 
   // Fill the range with ~'s.
-  StartColNo = map.byteToContainingColumn(StartColNo);
-  EndColNo = map.byteToContainingColumn(EndColNo);
+  StartColNo = Map.byteToContainingColumn(StartColNo);
+  EndColNo = Map.byteToContainingColumn(EndColNo);
 
   assert(StartColNo <= EndColNo && "Invalid range!");
   if (CaretLine.size() < EndColNo)
-    CaretLine.resize(EndColNo,' ');
-  std::fill(CaretLine.begin()+StartColNo,CaretLine.begin()+EndColNo,'~');
+    CaretLine.resize(EndColNo, ' ');
+  std::fill(CaretLine.begin() + StartColNo, CaretLine.begin() + EndColNo, '~');
 }
 
 static std::string buildFixItInsertionLine(FileID FID,
@@ -1100,6 +1056,57 @@ static unsigned getNumDisplayWidth(unsigned N) {
   return L;
 }
 
+/// Filter out invalid ranges, ranges that don't fit into the window of
+/// source lines we will print, and ranges from other files.
+///
+/// For the remaining ranges, convert them to simple LineRange structs,
+/// which only cover one line at a time.
+static SmallVector<LineRange>
+prepareAndFilterRanges(const SmallVectorImpl<CharSourceRange> &Ranges,
+                       const SourceManager &SM,
+                       const std::pair<unsigned, unsigned> &Lines, FileID FID,
+                       const LangOptions &LangOpts) {
+  SmallVector<LineRange> LineRanges;
+
+  for (const CharSourceRange &R : Ranges) {
+    if (R.isInvalid())
+      continue;
+    SourceLocation Begin = R.getBegin();
+    SourceLocation End = R.getEnd();
+
+    unsigned StartLineNo = SM.getExpansionLineNumber(Begin);
+    if (StartLineNo > Lines.second || SM.getFileID(Begin) != FID)
+      continue;
+
+    unsigned EndLineNo = SM.getExpansionLineNumber(End);
+    if (EndLineNo < Lines.first || SM.getFileID(End) != FID)
+      continue;
+
+    unsigned StartColumn = SM.getExpansionColumnNumber(Begin);
+    unsigned EndColumn = SM.getExpansionColumnNumber(End);
+    if (R.isTokenRange())
+      EndColumn += Lexer::MeasureTokenLength(End, SM, LangOpts);
+
+    // Only a single line.
+    if (StartLineNo == EndLineNo) {
+      LineRanges.push_back({StartLineNo, StartColumn - 1, EndColumn - 1});
+      continue;
+    }
+
+    // Start line.
+    LineRanges.push_back({StartLineNo, StartColumn - 1, ~0u});
+
+    // Middle lines.
+    for (unsigned S = StartLineNo + 1; S != EndLineNo; ++S)
+      LineRanges.push_back({S, 0, ~0u});
+
+    // End line.
+    LineRanges.push_back({EndLineNo, 0, EndColumn - 1});
+  }
+
+  return LineRanges;
+}
+
 /// Emit a code snippet and caret line.
 ///
 /// This routine emits a single line's code snippet and caret line..
@@ -1166,6 +1173,9 @@ void TextDiagnostic::emitSnippetAndCaret(
       OS.indent(MaxLineNoDisplayWidth + 2) << "| ";
   };
 
+  SmallVector<LineRange> LineRanges =
+      prepareAndFilterRanges(Ranges, SM, Lines, FID, LangOpts);
+
   for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1;
        ++LineNo, ++DisplayLineNo) {
     // Rewind from the current position to the start of the line.
@@ -1197,8 +1207,10 @@ void TextDiagnostic::emitSnippetAndCaret(
 
     std::string CaretLine;
     // Highlight all of the characters covered by Ranges with ~ characters.
-    for (const auto &I : Ranges)
-      highlightRange(I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
+    for (const auto &LR : LineRanges) {
+      if (LR.LineNo == LineNo)
+        highlightRange(LR, sourceColMap, CaretLine);
+    }
 
     // Next, insert the caret itself.
     if (CaretLineNo == LineNo) {

diff --git a/clang/test/Misc/caret-diags-multiline.cpp b/clang/test/Misc/caret-diags-multiline.cpp
index baf8e5a..15368fa 100644 (file)
--- a/clang/test/Misc/caret-diags-multiline.cpp
+++ b/clang/test/Misc/caret-diags-multiline.cpp
@@ -14,9 +14,9 @@ void line(int);
 // CHECK-NEXT: {{^}}  if (cond) {
 // CHECK-NEXT: {{^}}  ^~~~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(1);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}  } else {
-// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}  ~~~~~~{{$}}
 // CHECK-NEXT: note: initialize the variable
 int f1(int cond) {
   int a;
@@ -38,11 +38,11 @@ int f1(int cond) {
 // CHECK-NEXT: {{^}}  if (cond) {
 // CHECK-NEXT: {{^}}  ^~~~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(1);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(2);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}  } else {
-// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}  ~~~~~~{{$}}
 // CHECK-NEXT: note: initialize the variable
 int f2(int cond) {
   int a;
@@ -65,13 +65,13 @@ int f2(int cond) {
 // CHECK-NEXT: {{^}}  if (cond) {
 // CHECK-NEXT: {{^}}  ^~~~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(1);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(2);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(3);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}  } else {
-// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}  ~~~~~~{{$}}
 // CHECK-NEXT: note: initialize the variable
 int f3(int cond) {
   int a;
@@ -95,13 +95,13 @@ int f3(int cond) {
 // CHECK-NEXT: {{^}}  if (cond) {
 // CHECK-NEXT: {{^}}  ^~~~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(1);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(2);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(3);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(4);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: note: initialize the variable
 int f4(int cond) {
   int a;
@@ -126,13 +126,13 @@ int f4(int cond) {
 // CHECK-NEXT: {{^}}  if (cond) {
 // CHECK-NEXT: {{^}}  ^~~~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(1);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(2);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(3);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: {{^}}    line(4);
-// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
+// CHECK-NEXT: {{^}}    ~~~~~~~~{{$}}
 // CHECK-NEXT: note: initialize the variable
 int f5(int cond) {
   int a;