[PATCH] D151215: [clang][Diagnostics] Split source ranges into line ranges before...

2023-06-02 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder updated this revision to Diff 527766.
tbaeder added a comment.

@cor3ntin I added changes to the two brackets tests in `test/Parser/`. They 
look like an improvement to me, but maybe there's something I'm missing, so 
please have a quick look.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151215/new/

https://reviews.llvm.org/D151215

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

Index: clang/test/Parser/brackets.cpp
===
--- clang/test/Parser/brackets.cpp
+++ clang/test/Parser/brackets.cpp
@@ -33,7 +33,7 @@
   int [3] (*a) = 0;
   // expected-error@-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 @@
 int [5] *B::x = 0;
 // expected-error@-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 @@
   int [3] *a;
   // expected-error@-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 @@
   int [2] a;
   // expected-error@-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 @@
   int [2]  = a;
   // expected-error@-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]]:11-[[@LINE-6]]:11}:"("
@@ -130,7 +130,7 @@
 int [3] ::test6::A::arr = {1,2,3};
 // expected-error@-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 @@
   int [3] A::*a;
   // expected-error@-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}:"("
Index: clang/test/Parser/brackets.c
===
--- clang/test/Parser/brackets.c
+++ clang/test/Parser/brackets.c
@@ -55,7 +55,7 @@
   // CHECK-NOT: fix-it
   // expected-error@-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 @@
   int [5] * a;
   // expected-error@-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}:"("
Index: clang/test/Misc/caret-diags-multiline.cpp
===
--- clang/test/Misc/caret-diags-multiline.cpp
+++ clang/test/Misc/caret-diags-multiline.cpp
@@ -14,9 +14,9 @@
 // CHECK-NEXT: {{^}}  if (cond) {
 // CHECK-NEXT: {{^}}  ^~~{{$}}
 // CHECK-NEXT: {{^}}line(1);
-// CHECK-NEXT: {{^}}{{$}}
+// CHECK-NEXT: {{^}}{{$}}

[PATCH] D151215: [clang][Diagnostics] Split source ranges into line ranges before...

2023-06-02 Thread Timm Bäder via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc1262bd58ac: [clang][Diagnostics] Split source ranges into 
line ranges before... (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D151215?vs=524713=527760#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151215/new/

https://reviews.llvm.org/D151215

Files:
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Misc/caret-diags-multiline.cpp

Index: clang/test/Misc/caret-diags-multiline.cpp
===
--- clang/test/Misc/caret-diags-multiline.cpp
+++ clang/test/Misc/caret-diags-multiline.cpp
@@ -14,9 +14,9 @@
 // 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 @@
 // 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 @@
 // 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 @@
 // 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 @@
 // 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;
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -945,87 +945,43 @@
   return A;
 }
 
-/// Highlight a SourceRange (with ~'s) for any characters on LineNo.
-static void highlightRange(const CharSourceRange ,
-   unsigned LineNo, FileID FID,
-   const SourceColumnMap ,
-   std::string ,
-   const SourceManager ,
-   const LangOptions ) {
-  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 #.

[PATCH] D151215: [clang][Diagnostics] Split source ranges into line ranges before...

2023-06-02 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin accepted this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.

Looks good to me too!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151215/new/

https://reviews.llvm.org/D151215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151215: [clang][Diagnostics] Split source ranges into line ranges before...

2023-05-25 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added reviewers: tahonermann, cor3ntin.
aaron.ballman added subscribers: cor3ntin, tahonermann.
aaron.ballman added a comment.

This looks correct to me, but because it involves things like characters, 
lines, and output, I'd appreciate if @tahonermann or @cor3ntin could do the 
final sign-off just in case I missed something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151215/new/

https://reviews.llvm.org/D151215

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151215: [clang][Diagnostics] Split source ranges into line ranges before...

2023-05-23 Thread Timm Bäder via Phabricator via cfe-commits 
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, cjdb, clang.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  ... 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151215

Files:
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Misc/caret-diags-multiline.cpp

Index: clang/test/Misc/caret-diags-multiline.cpp
===
--- clang/test/Misc/caret-diags-multiline.cpp
+++ clang/test/Misc/caret-diags-multiline.cpp
@@ -14,9 +14,9 @@
 // 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 @@
 // 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 @@
 // 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 @@
 // 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 @@
 // 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;
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -954,87 +954,43 @@
   return A;
 }
 
-/// Highlight a SourceRange (with ~'s) for any characters on LineNo.
-static void highlightRange(const CharSourceRange ,
-   unsigned LineNo, FileID FID,
-   const SourceColumnMap ,
-   std::string ,
-   const SourceManager ,
-   const LangOptions ) {
-  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