Skip to content

Commit 997c2f7

Browse files
tbaederrtbaederr
committed
[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
1 parent af22be3 commit 997c2f7

File tree

4 files changed

+115
-103
lines changed

4 files changed

+115
-103
lines changed

‎clang/lib/Frontend/TextDiagnostic.cpp

Lines changed: 89 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -945,87 +945,43 @@ maybeAddRange(std::pair<unsigned, unsigned> A, std::pair<unsigned, unsigned> B,
945945
return A;
946946
}
947947

948-
/// Highlight a SourceRange (with ~'s) for any characters on LineNo.
949-
static void highlightRange(const CharSourceRange &R,
950-
unsigned LineNo, FileID FID,
951-
const SourceColumnMap &map,
952-
std::string &CaretLine,
953-
const SourceManager &SM,
954-
const LangOptions &LangOpts) {
955-
if (!R.isValid()) return;
956-
957-
SourceLocation Begin = R.getBegin();
958-
SourceLocation End = R.getEnd();
959-
960-
unsigned StartLineNo = SM.getExpansionLineNumber(Begin);
961-
if (StartLineNo > LineNo || SM.getFileID(Begin) != FID)
962-
return; // No intersection.
963-
964-
unsigned EndLineNo = SM.getExpansionLineNumber(End);
965-
if (EndLineNo < LineNo || SM.getFileID(End) != FID)
966-
return; // No intersection.
967-
968-
// Compute the column number of the start.
969-
unsigned StartColNo = 0;
970-
if (StartLineNo == LineNo) {
971-
StartColNo = SM.getExpansionColumnNumber(Begin);
972-
if (StartColNo) --StartColNo; // Zero base the col #.
973-
}
974-
975-
// Compute the column number of the end.
976-
unsigned EndColNo = map.getSourceLine().size();
977-
if (EndLineNo == LineNo) {
978-
EndColNo = SM.getExpansionColumnNumber(End);
979-
if (EndColNo) {
980-
--EndColNo; // Zero base the col #.
981-
982-
// Add in the length of the token, so that we cover multi-char tokens if
983-
// this is a token range.
984-
if (R.isTokenRange())
985-
EndColNo += Lexer::MeasureTokenLength(End, SM, LangOpts);
986-
} else {
987-
EndColNo = CaretLine.size();
988-
}
989-
}
990-
991-
assert(StartColNo <= EndColNo && "Invalid range!");
992-
993-
// Check that a token range does not highlight only whitespace.
994-
if (R.isTokenRange()) {
995-
// Pick the first non-whitespace column.
996-
while (StartColNo < map.getSourceLine().size() &&
997-
(map.getSourceLine()[StartColNo] == ' ' ||
998-
map.getSourceLine()[StartColNo] == '\t'))
999-
StartColNo = map.startOfNextColumn(StartColNo);
1000-
1001-
// Pick the last non-whitespace column.
1002-
if (EndColNo > map.getSourceLine().size())
1003-
EndColNo = map.getSourceLine().size();
1004-
while (EndColNo &&
1005-
(map.getSourceLine()[EndColNo-1] == ' ' ||
1006-
map.getSourceLine()[EndColNo-1] == '\t'))
1007-
EndColNo = map.startOfPreviousColumn(EndColNo);
1008-
1009-
// If the start/end passed each other, then we are trying to highlight a
1010-
// range that just exists in whitespace. That most likely means we have
1011-
// a multi-line highlighting range that covers a blank line.
1012-
if (StartColNo > EndColNo) {
1013-
assert(StartLineNo != EndLineNo && "trying to highlight whitespace");
1014-
StartColNo = EndColNo;
1015-
}
1016-
}
948+
struct LineRange {
949+
unsigned LineNo;
950+
unsigned StartCol;
951+
unsigned EndCol;
952+
};
1017953

1018-
assert(StartColNo <= map.getSourceLine().size() && "Invalid range!");
1019-
assert(EndColNo <= map.getSourceLine().size() && "Invalid range!");
954+
/// Highlight \p R (with ~'s) on the current source line.
955+
static void highlightRange(const LineRange &R, const SourceColumnMap &Map,
956+
std::string &CaretLine) {
957+
// Pick the first non-whitespace column.
958+
unsigned StartColNo = R.StartCol;
959+
while (StartColNo < Map.getSourceLine().size() &&
960+
(Map.getSourceLine()[StartColNo] == ' ' ||
961+
Map.getSourceLine()[StartColNo] == '\t'))
962+
StartColNo = Map.startOfNextColumn(StartColNo);
963+
964+
// Pick the last non-whitespace column.
965+
unsigned EndColNo =
966+
std::min(static_cast<size_t>(R.EndCol), Map.getSourceLine().size());
967+
while (EndColNo && (Map.getSourceLine()[EndColNo - 1] == ' ' ||
968+
Map.getSourceLine()[EndColNo - 1] == '\t'))
969+
EndColNo = Map.startOfPreviousColumn(EndColNo);
970+
971+
// If the start/end passed each other, then we are trying to highlight a
972+
// range that just exists in whitespace. That most likely means we have
973+
// a multi-line highlighting range that covers a blank line.
974+
if (StartColNo > EndColNo)
975+
return;
1020976

1021977
// Fill the range with ~'s.
1022-
StartColNo = map.byteToContainingColumn(StartColNo);
1023-
EndColNo = map.byteToContainingColumn(EndColNo);
978+
StartColNo = Map.byteToContainingColumn(StartColNo);
979+
EndColNo = Map.byteToContainingColumn(EndColNo);
1024980

1025981
assert(StartColNo <= EndColNo && "Invalid range!");
1026982
if (CaretLine.size() < EndColNo)
1027-
CaretLine.resize(EndColNo,' ');
1028-
std::fill(CaretLine.begin()+StartColNo,CaretLine.begin()+EndColNo,'~');
983+
CaretLine.resize(EndColNo, ' ');
984+
std::fill(CaretLine.begin() + StartColNo, CaretLine.begin() + EndColNo, '~');
1029985
}
1030986

1031987
static std::string buildFixItInsertionLine(FileID FID,
@@ -1100,6 +1056,57 @@ static unsigned getNumDisplayWidth(unsigned N) {
11001056
return L;
11011057
}
11021058

1059+
/// Filter out invalid ranges, ranges that don't fit into the window of
1060+
/// source lines we will print, and ranges from other files.
1061+
///
1062+
/// For the remaining ranges, convert them to simple LineRange structs,
1063+
/// which only cover one line at a time.
1064+
static SmallVector<LineRange>
1065+
prepareAndFilterRanges(const SmallVectorImpl<CharSourceRange> &Ranges,
1066+
const SourceManager &SM,
1067+
const std::pair<unsigned, unsigned> &Lines, FileID FID,
1068+
const LangOptions &LangOpts) {
1069+
SmallVector<LineRange> LineRanges;
1070+
1071+
for (const CharSourceRange &R : Ranges) {
1072+
if (R.isInvalid())
1073+
continue;
1074+
SourceLocation Begin = R.getBegin();
1075+
SourceLocation End = R.getEnd();
1076+
1077+
unsigned StartLineNo = SM.getExpansionLineNumber(Begin);
1078+
if (StartLineNo > Lines.second || SM.getFileID(Begin) != FID)
1079+
continue;
1080+
1081+
unsigned EndLineNo = SM.getExpansionLineNumber(End);
1082+
if (EndLineNo < Lines.first || SM.getFileID(End) != FID)
1083+
continue;
1084+
1085+
unsigned StartColumn = SM.getExpansionColumnNumber(Begin);
1086+
unsigned EndColumn = SM.getExpansionColumnNumber(End);
1087+
if (R.isTokenRange())
1088+
EndColumn += Lexer::MeasureTokenLength(End, SM, LangOpts);
1089+
1090+
// Only a single line.
1091+
if (StartLineNo == EndLineNo) {
1092+
LineRanges.push_back({StartLineNo, StartColumn - 1, EndColumn - 1});
1093+
continue;
1094+
}
1095+
1096+
// Start line.
1097+
LineRanges.push_back({StartLineNo, StartColumn - 1, ~0u});
1098+
1099+
// Middle lines.
1100+
for (unsigned S = StartLineNo + 1; S != EndLineNo; ++S)
1101+
LineRanges.push_back({S, 0, ~0u});
1102+
1103+
// End line.
1104+
LineRanges.push_back({EndLineNo, 0, EndColumn - 1});
1105+
}
1106+
1107+
return LineRanges;
1108+
}
1109+
11031110
/// Emit a code snippet and caret line.
11041111
///
11051112
/// This routine emits a single line's code snippet and caret line..
@@ -1166,6 +1173,9 @@ void TextDiagnostic::emitSnippetAndCaret(
11661173
OS.indent(MaxLineNoDisplayWidth + 2) << "| ";
11671174
};
11681175

1176+
SmallVector<LineRange> LineRanges =
1177+
prepareAndFilterRanges(Ranges, SM, Lines, FID, LangOpts);
1178+
11691179
for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1;
11701180
++LineNo, ++DisplayLineNo) {
11711181
// Rewind from the current position to the start of the line.
@@ -1197,8 +1207,10 @@ void TextDiagnostic::emitSnippetAndCaret(
11971207

11981208
std::string CaretLine;
11991209
// Highlight all of the characters covered by Ranges with ~ characters.
1200-
for (const auto &I : Ranges)
1201-
highlightRange(I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
1210+
for (const auto &LR : LineRanges) {
1211+
if (LR.LineNo == LineNo)
1212+
highlightRange(LR, sourceColMap, CaretLine);
1213+
}
12021214

12031215
// Next, insert the caret itself.
12041216
if (CaretLineNo == LineNo) {

‎clang/test/Misc/caret-diags-multiline.cpp

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ void line(int);
1414
// CHECK-NEXT: {{^}} if (cond) {
1515
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
1616
// CHECK-NEXT: {{^}} line(1);
17-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
17+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
1818
// CHECK-NEXT: {{^}} } else {
19-
// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
19+
// CHECK-NEXT: {{^}} ~~~~~~{{$}}
2020
// CHECK-NEXT: note: initialize the variable
2121
int f1(int cond) {
2222
int a;
@@ -38,11 +38,11 @@ int f1(int cond) {
3838
// CHECK-NEXT: {{^}} if (cond) {
3939
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
4040
// CHECK-NEXT: {{^}} line(1);
41-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
41+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
4242
// CHECK-NEXT: {{^}} line(2);
43-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
43+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
4444
// CHECK-NEXT: {{^}} } else {
45-
// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
45+
// CHECK-NEXT: {{^}} ~~~~~~{{$}}
4646
// CHECK-NEXT: note: initialize the variable
4747
int f2(int cond) {
4848
int a;
@@ -65,13 +65,13 @@ int f2(int cond) {
6565
// CHECK-NEXT: {{^}} if (cond) {
6666
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
6767
// CHECK-NEXT: {{^}} line(1);
68-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
68+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
6969
// CHECK-NEXT: {{^}} line(2);
70-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
70+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
7171
// CHECK-NEXT: {{^}} line(3);
72-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
72+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
7373
// CHECK-NEXT: {{^}} } else {
74-
// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
74+
// CHECK-NEXT: {{^}} ~~~~~~{{$}}
7575
// CHECK-NEXT: note: initialize the variable
7676
int f3(int cond) {
7777
int a;
@@ -95,13 +95,13 @@ int f3(int cond) {
9595
// CHECK-NEXT: {{^}} if (cond) {
9696
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
9797
// CHECK-NEXT: {{^}} line(1);
98-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
98+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
9999
// CHECK-NEXT: {{^}} line(2);
100-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
100+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
101101
// CHECK-NEXT: {{^}} line(3);
102-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
102+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
103103
// CHECK-NEXT: {{^}} line(4);
104-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
104+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
105105
// CHECK-NEXT: note: initialize the variable
106106
int f4(int cond) {
107107
int a;
@@ -126,13 +126,13 @@ int f4(int cond) {
126126
// CHECK-NEXT: {{^}} if (cond) {
127127
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
128128
// CHECK-NEXT: {{^}} line(1);
129-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
129+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
130130
// CHECK-NEXT: {{^}} line(2);
131-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
131+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
132132
// CHECK-NEXT: {{^}} line(3);
133-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
133+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
134134
// CHECK-NEXT: {{^}} line(4);
135-
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
135+
// CHECK-NEXT: {{^}} ~~~~~~~~{{$}}
136136
// CHECK-NEXT: note: initialize the variable
137137
int f5(int cond) {
138138
int a;

‎clang/test/Parser/brackets.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ void test3(void) {
5555
// CHECK-NOT: fix-it
5656
// expected-error@-5{{brackets are not allowed here; to declare an array, place the brackets after the identifier}}
5757
// CHECK: {{^}} int [5] *;
58-
// CHECK: {{^}} ~~~~ ^
58+
// CHECK: {{^}} ~~~ ^
5959
// CHECK: {{^}} ()[5]
6060
// CHECK: fix-it:{{.*}}:{[[@LINE-9]]:7-[[@LINE-9]]:11}:""
6161
// CHECK: fix-it:{{.*}}:{[[@LINE-10]]:11-[[@LINE-10]]:11}:"("
@@ -64,7 +64,7 @@ void test3(void) {
6464
int [5] * a;
6565
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the identifier}}
6666
// CHECK: {{^}} int [5] * a;
67-
// CHECK: {{^}} ~~~~ ^
67+
// CHECK: {{^}} ~~~ ^
6868
// CHECK: {{^}} ( )[5]
6969
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
7070
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("

‎clang/test/Parser/brackets.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ void test2() {
3333
int [3] (*a) = 0;
3434
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
3535
// CHECK: {{^}} int [3] (*a) = 0;
36-
// CHECK: {{^}} ~~~~ ^
36+
// CHECK: {{^}} ~~~ ^
3737
// CHECK: {{^}} [3]
3838
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
3939
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:15-[[@LINE-6]]:15}:"[3]"
@@ -67,7 +67,7 @@ struct B { static int (*x)[5]; };
6767
int [5] *B::x = 0;
6868
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
6969
// CHECK: {{^}}int [5] *B::x = 0;
70-
// CHECK: {{^}} ~~~~ ^
70+
// CHECK: {{^}} ~~~ ^
7171
// CHECK: {{^}} ( )[5]
7272
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:9}:""
7373
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:9-[[@LINE-6]]:9}:"("
@@ -77,7 +77,7 @@ void test3() {
7777
int [3] *a;
7878
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
7979
// CHECK: {{^}} int [3] *a;
80-
// CHECK: {{^}} ~~~~ ^
80+
// CHECK: {{^}} ~~~ ^
8181
// CHECK: {{^}} ( )[3]
8282
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
8383
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
@@ -90,15 +90,15 @@ void test4() {
9090
int [2] a;
9191
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
9292
// CHECK: {{^}} int [2] a;
93-
// CHECK: {{^}} ~~~~ ^
93+
// CHECK: {{^}} ~~~ ^
9494
// CHECK: {{^}} [2]
9595
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
9696
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:12-[[@LINE-6]]:12}:"[2]"
9797

9898
int [2] &b = a;
9999
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
100100
// CHECK: {{^}} int [2] &b = a;
101-
// CHECK: {{^}} ~~~~ ^
101+
// CHECK: {{^}} ~~~ ^
102102
// CHECK: {{^}} ( )[2]
103103
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
104104
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("
@@ -130,7 +130,7 @@ struct A {
130130
int [3] ::test6::A::arr = {1,2,3};
131131
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
132132
// CHECK: {{^}}int [3] ::test6::A::arr = {1,2,3};
133-
// CHECK: {{^}} ~~~~ ^
133+
// CHECK: {{^}} ~~~ ^
134134
// CHECK: {{^}} [3]
135135
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:5-[[@LINE-5]]:9}:""
136136
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:24-[[@LINE-6]]:24}:"[3]"
@@ -143,7 +143,7 @@ void test() {
143143
int [3] A::*a;
144144
// expected-error@-1{{brackets are not allowed here; to declare an array, place the brackets after the name}}
145145
// CHECK: {{^}} int [3] A::*a;
146-
// CHECK: {{^}} ~~~~ ^
146+
// CHECK: {{^}} ~~~ ^
147147
// CHECK: {{^}} ( )[3]
148148
// CHECK: fix-it:{{.*}}:{[[@LINE-5]]:7-[[@LINE-5]]:11}:""
149149
// CHECK: fix-it:{{.*}}:{[[@LINE-6]]:11-[[@LINE-6]]:11}:"("

0 commit comments

Comments
 (0)