[clang] 997c2f7 - [clang][Diagnostics] Split source ranges into line ranges before...

Timm Bäder via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 5 05:57:16 PDT 2023


Author: Timm Bäder
Date: 2023-06-05T14:56:58+02:00
New Revision: 997c2f70c1a29f911e559919f4799cd2b341796f

URL: https://github.com/llvm/llvm-project/commit/997c2f70c1a29f911e559919f4799cd2b341796f
DIFF: https://github.com/llvm/llvm-project/commit/997c2f70c1a29f911e559919f4799cd2b341796f.diff

LOG: [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

Added: 
    

Modified: 
    clang/lib/Frontend/TextDiagnostic.cpp
    clang/test/Misc/caret-diags-multiline.cpp
    clang/test/Parser/brackets.c
    clang/test/Parser/brackets.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp
index 137001dc050d1..01d2b10479ade 100644
--- 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 baf8e5a219be3..15368faa65b6f 100644
--- 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;

diff  --git a/clang/test/Parser/brackets.c b/clang/test/Parser/brackets.c
index 1821747d13515..78ac553130ab4 100644
--- a/clang/test/Parser/brackets.c
+++ b/clang/test/Parser/brackets.c
@@ -55,7 +55,7 @@ void test3(void) {
   // CHECK-NOT: fix-it
   // expected-error at -5{{brackets are not allowed here; to declare an array, place the brackets after the identifier}}
   // CHECK: {{^}}  int [5] *;
-  // CHECK: {{^}}      ~~~~ ^
+  // CHECK: {{^}}      ~~~  ^
   // CHECK: {{^}}          ()[5]
   // CHECK: fix-it:{{.*}}:{[[@LINE-9]]:7-[[@LINE-9]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-10]]:11-[[@LINE-10]]:11}:"("
@@ -64,7 +64,7 @@ void test3(void) {
   int [5] * a;
   // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the identifier}}
   // CHECK: {{^}}  int [5] * a;
-  // CHECK: {{^}}      ~~~~   ^
+  // CHECK: {{^}}      ~~~    ^
   // CHECK: {{^}}          (  )[5]
   // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("

diff  --git a/clang/test/Parser/brackets.cpp b/clang/test/Parser/brackets.cpp
index 40b08c37a06a1..927b66a7ebcb8 100644
--- a/clang/test/Parser/brackets.cpp
+++ b/clang/test/Parser/brackets.cpp
@@ -33,7 +33,7 @@ void test2() {
   int [3] (*a) = 0;
   // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
   // CHECK: {{^}}  int [3] (*a) = 0;
-  // CHECK: {{^}}      ~~~~    ^
+  // CHECK: {{^}}      ~~~     ^
   // CHECK: {{^}}              [3]
   // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:15-[[@LINE-6]]:15}:"[3]"
@@ -67,7 +67,7 @@ struct B { static int (*x)[5]; };
 int [5] *B::x = 0;
 // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
 // CHECK: {{^}}int [5] *B::x = 0;
-// CHECK: {{^}}    ~~~~     ^
+// CHECK: {{^}}    ~~~      ^
 // CHECK: {{^}}        (    )[5]
 // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:9}:""
 // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:9-[[@LINE-6]]:9}:"("
@@ -77,7 +77,7 @@ void test3() {
   int [3] *a;
   // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
   // CHECK: {{^}}  int [3] *a;
-  // CHECK: {{^}}      ~~~~  ^
+  // CHECK: {{^}}      ~~~   ^
   // CHECK: {{^}}          ( )[3]
   // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
@@ -90,7 +90,7 @@ void test4() {
   int [2] a;
   // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
   // CHECK: {{^}}  int [2] a;
-  // CHECK: {{^}}      ~~~~ ^
+  // CHECK: {{^}}      ~~~  ^
   // CHECK: {{^}}           [2]
   // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:12-[[@LINE-6]]:12}:"[2]"
@@ -98,7 +98,7 @@ void test4() {
   int [2] &b = a;
   // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
   // CHECK: {{^}}  int [2] &b = a;
-  // CHECK: {{^}}      ~~~~  ^
+  // CHECK: {{^}}      ~~~   ^
   // CHECK: {{^}}          ( )[2]
   // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
@@ -130,7 +130,7 @@ struct A {
 int [3] ::test6::A::arr = {1,2,3};
 // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
 // CHECK: {{^}}int [3] ::test6::A::arr = {1,2,3};
-// CHECK: {{^}}    ~~~~               ^
+// CHECK: {{^}}    ~~~                ^
 // CHECK: {{^}}                       [3]
 // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:9}:""
 // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:24-[[@LINE-6]]:24}:"[3]"
@@ -143,7 +143,7 @@ void test() {
   int [3] A::*a;
   // expected-error at -1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
   // CHECK: {{^}}  int [3] A::*a;
-  // CHECK: {{^}}      ~~~~     ^
+  // CHECK: {{^}}      ~~~      ^
   // CHECK: {{^}}          (    )[3]
   // CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
   // CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("


        


More information about the cfe-commits mailing list