Skip to content

Commit 0db205b

Browse files
ojhuntojhunt
committed
[clang] Correct FixIt ranges for unused capture warnings
Fixes #106445 by using the lexer to find the correct range for the removal FixIts. Previously the ranges that were generated assuming no unsurprising formatting, which for the most part works. Being correct in all cases requires using the lexer to find the bounding tokens for the region to remove. As part of this it adds Sema::getRangeForNextToken to wrap Lexer::findNextToken.
1 parent 6212c19 commit 0db205b

File tree

4 files changed

+291
-6
lines changed

4 files changed

+291
-6
lines changed

‎clang/include/clang/Sema/Sema.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,12 @@ class Sema final : public SemaBase {
972972
/// Calls \c Lexer::getLocForEndOfToken()
973973
SourceLocation getLocForEndOfToken(SourceLocation Loc, unsigned Offset = 0);
974974

975+
/// Calls \c Lexer::findNextToken() to find the next token, and if the
976+
/// locations of both ends of the token can be resolved it return that
977+
/// range; Otherwise it returns an invalid SourceRange.
978+
SourceRange getRangeForNextToken(SourceLocation Loc,
979+
bool IncludeComments = false);
980+
975981
/// Retrieve the module loader associated with the preprocessor.
976982
ModuleLoader &getModuleLoader() const;
977983

‎clang/lib/Sema/Sema.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,21 @@ SourceLocation Sema::getLocForEndOfToken(SourceLocation Loc, unsigned Offset) {
8484
return Lexer::getLocForEndOfToken(Loc, Offset, SourceMgr, LangOpts);
8585
}
8686

87+
SourceRange Sema::getRangeForNextToken(SourceLocation Loc,
88+
bool IncludeComments) {
89+
if (!Loc.isValid())
90+
return SourceRange();
91+
std::optional<Token> NextToken =
92+
Lexer::findNextToken(Loc, SourceMgr, LangOpts, IncludeComments);
93+
if (!NextToken)
94+
return SourceRange();
95+
SourceLocation TokenStart = NextToken->getLocation();
96+
SourceLocation TokenEnd = NextToken->getLastLoc();
97+
if (!TokenStart.isValid() || !TokenEnd.isValid())
98+
return SourceRange();
99+
return SourceRange(TokenStart, TokenEnd);
100+
}
101+
87102
ModuleLoader &Sema::getModuleLoader() const { return PP.getModuleLoader(); }
88103

89104
DarwinSDKInfo *

‎clang/lib/Sema/SemaLambda.cpp‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,15 +2164,29 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
21642164
bool IsLast = (I + 1) == LSI->NumExplicitCaptures;
21652165
SourceRange FixItRange;
21662166
if (CaptureRange.isValid()) {
2167+
auto GetTrailingEndLocation = [&](SourceLocation StartPoint) {
2168+
SourceRange NextToken =
2169+
getRangeForNextToken(StartPoint, /*IncludeComments=*/true);
2170+
if (!NextToken.isValid())
2171+
return SourceLocation();
2172+
// Return the last location preceding the next token
2173+
return NextToken.getBegin().getLocWithOffset(-1);
2174+
};
21672175
if (!CurHasPreviousCapture && !IsLast) {
21682176
// If there are no captures preceding this capture, remove the
2169-
// following comma.
2170-
FixItRange = SourceRange(CaptureRange.getBegin(),
2171-
getLocForEndOfToken(CaptureRange.getEnd()));
2177+
// trailing comma and anything up to the next token
2178+
SourceRange CommaRange =
2179+
getRangeForNextToken(CaptureRange.getEnd());
2180+
SourceLocation FixItEnd =
2181+
GetTrailingEndLocation(CommaRange.getBegin());
2182+
FixItRange = SourceRange(CaptureRange.getBegin(), FixItEnd);
21722183
} else {
2173-
// Otherwise, remove the comma since the last used capture.
2174-
FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc),
2175-
CaptureRange.getEnd());
2184+
// Otherwise, remove the comma since the last used capture, and
2185+
// anything up to the next token
2186+
SourceLocation FixItStart = getLocForEndOfToken(PrevCaptureLoc);
2187+
SourceLocation FixItEnd =
2188+
GetTrailingEndLocation(CaptureRange.getEnd());
2189+
FixItRange = SourceRange(FixItStart, FixItEnd);
21762190
}
21772191
}
21782192

‎clang/test/FixIt/fixit-unused-lambda-capture-trailing-tokens.cpp‎

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
// RUN: cp %s %t
2+
// RUN: %clang_cc1 -x c++ -Wunused-lambda-capture -Wno-unused-value -std=c++1z -fixit %t
3+
// RUN: grep -v CHECK %t | FileCheck %s
4+
5+
6+
#define MACRO_CAPTURE(...) __VA_ARGS__
7+
int main() {
8+
int a = 0, b = 0, c = 0;
9+
auto F0 = [a, &b]() mutable {
10+
// CHECK: auto F0 = [a]()
11+
a++;
12+
};
13+
14+
auto F1 = [&a, &b]() {
15+
// CHECK: auto F1 = []() {
16+
};
17+
18+
auto F2 = [&a, b]() {
19+
// CHECK: auto F2 = []() {
20+
};
21+
22+
auto F3 = [&a,
23+
&b]() {
24+
// CHECK: auto F3 = []() {
25+
};
26+
27+
auto F4 = [&a
28+
, &b]() {
29+
// CHECK: auto F4 = []() {
30+
};
31+
auto F5 = [&a ,&b]() {
32+
// CHECK: auto F5 = []() {
33+
};
34+
35+
auto F0a = [a, &b]() mutable {
36+
// CHECK: auto F0a = [a]() mutable {
37+
a++;
38+
};
39+
40+
auto F1a = [&a, &b]() {
41+
// CHECK: auto F1a = [&a]() {
42+
a++;
43+
};
44+
45+
auto F2a = [&a, b]() {
46+
// CHECK: auto F2a = [&a]() {
47+
a++;
48+
};
49+
50+
auto F3a = [&a,
51+
&b]() {
52+
// CHECK: auto F3a = [&a]() {
53+
a++;
54+
};
55+
56+
auto F4a = [&a
57+
, &b]() {
58+
// CHECK: auto F4a = [&a]() {
59+
a++;
60+
};
61+
62+
auto F5a = [&a ,&b]() {
63+
// CHECK: auto F5a = [&a]() {
64+
a++;
65+
};
66+
auto F0b = [a, &b]() mutable {
67+
// CHECK: auto F0b = [ &b]() mutable
68+
b++;
69+
};
70+
71+
auto F1b = [&a, &b]() {
72+
// CHECK: auto F1b = [ &b]() {
73+
b++;
74+
};
75+
76+
auto F2b = [&a, b]() mutable {
77+
// CHECK: auto F2b = [ b]() mutable {
78+
b++;
79+
};
80+
81+
auto F3b = [&a,
82+
&b]() {
83+
// CHECK: auto F3b = [ &b]() {
84+
b++;
85+
};
86+
87+
auto F4b = [&a
88+
, &b]() {
89+
// CHECK: auto F4b = [ &b]() {
90+
b++;
91+
};
92+
auto F5b = [&a ,&b]() {
93+
// CHECK: auto F5b = [&b]() {
94+
b++;
95+
};
96+
97+
auto F6 = [&a, &b, &c]() {
98+
// CHECK: auto F6 = [&a, &b]() {
99+
a++;
100+
b++;
101+
};
102+
auto F7 = [&a, &b, &c]() {
103+
// CHECK: auto F7 = [&a, &c]() {
104+
a++;
105+
c++;
106+
};
107+
auto F8 = [&a, &b, &c]() {
108+
// CHECK: auto F8 = [ &b, &c]() {
109+
b++;
110+
c++;
111+
};
112+
auto F9 = [&a, &b , &c]() {
113+
// CHECK: auto F9 = [&a , &c]() {
114+
a++;
115+
c++;
116+
};
117+
auto F10 = [&a,
118+
&b, &c]() {
119+
// CHECK: auto F10 = [&a, &c]() {
120+
a++;
121+
c++;
122+
};
123+
auto F11 = [&a, &b ,
124+
&c]() {
125+
// CHECK: auto F11 = [&a ,
126+
// CHECK-NEXT: &c]() {
127+
a++;
128+
c++;
129+
};
130+
auto F12 = [a = 0, b ,
131+
c]() mutable {
132+
// CHECK: auto F12 = [ b ,
133+
// CHECK-NEXT: c]() mutable {
134+
b++;
135+
c++;
136+
};
137+
auto F13 = [a, b = 0 ,
138+
c]() mutable {
139+
// CHECK: auto F13 = [a ,
140+
// CHECK-NEXT: c]() mutable {
141+
a++;
142+
c++;
143+
};
144+
auto F14 = [a, b ,
145+
c
146+
= 0]() mutable {
147+
// CHECK: auto F14 = [a, b]() mutable {
148+
a++;
149+
b++;
150+
};
151+
152+
// We want to remove everything including the comment
153+
// as well as the comma following the capture of `a`
154+
auto F15 = [&a /* comment */, &b]() {
155+
// CHECK: auto F15 = [ &b]() {
156+
b++;
157+
};
158+
159+
// The comment preceding the first capture remains. This is more
160+
// by design of the fixit logic than anything else, but we should
161+
// consider the preceding comment might actually be a comment for
162+
// the entire capture set, so maybe we do want it to hang around
163+
auto F16 = [/* comment */ &a , &b]() {
164+
// CHECK: auto F16 = [/* comment */ &b]() {
165+
b++;
166+
};
167+
168+
auto F16b = [&a , /* comment */ &b]() {
169+
// CHECK: auto F16b = [ /* comment */ &b]() {
170+
b++;
171+
};
172+
173+
auto F17 = [&a /* comment */, &b]() {
174+
// CHECK: auto F17 = [&a]() {
175+
a++;
176+
};
177+
178+
auto F18 = [&a , /* comment */ &b]() {
179+
// CHECK: auto F18 = [&a]() {
180+
a++;
181+
};
182+
183+
auto F19 = [&a /* comment */, &b /* comment */]() {
184+
// CHECK: auto F19 = [&a /* comment */]() {
185+
a++;
186+
};
187+
188+
auto F20 = [MACRO_CAPTURE(&a, &b)]() {
189+
// CHECK: auto F20 = [MACRO_CAPTURE(&a, &b)]() {
190+
};
191+
192+
auto F21 = [MACRO_CAPTURE(&a), &b]() {
193+
// CHECK: auto F21 = []() {
194+
};
195+
196+
auto F22 = [MACRO_CAPTURE(&a,) &b]() {
197+
// CHECK: auto F22 = [MACRO_CAPTURE(&a,) &b]() {
198+
};
199+
auto F23 = [&a MACRO_CAPTURE(, &b)]() {
200+
// CHECK: auto F23 = []() {
201+
};
202+
auto F24 = [&a, MACRO_CAPTURE(&b)]() {
203+
// CHECK: auto F24 = []() {
204+
};
205+
206+
auto F20a = [MACRO_CAPTURE(&a, &b)]() {
207+
// CHECK: auto F20a = [MACRO_CAPTURE(&a, &b)]() {
208+
a++;
209+
};
210+
211+
auto F21a = [MACRO_CAPTURE(&a), &b]() {
212+
// CHECK: auto F21a = [MACRO_CAPTURE(&a)]() {
213+
a++;
214+
};
215+
216+
auto F22a = [MACRO_CAPTURE(&a,) &b]() {
217+
// Cauto F22a = [MACRO_CAPTURE(&a,) &b]() {
218+
a++;
219+
};
220+
auto F23a = [&a MACRO_CAPTURE(, &b)]() {
221+
// CHECK: auto F23a = [&a]() {
222+
a++;
223+
};
224+
auto F24a = [&a, MACRO_CAPTURE(&b)]() {
225+
// CHECK: auto F24a = [&a]() {
226+
a++;
227+
};
228+
auto F20b = [MACRO_CAPTURE(&a, &b)]() {
229+
// CHECK: auto F20b = [MACRO_CAPTURE(&a, &b)]() {
230+
b++;
231+
};
232+
233+
auto F21b = [MACRO_CAPTURE(&a), &b]() {
234+
// CHECK: auto F21b = [ &b]() {
235+
b++;
236+
};
237+
238+
auto F22b = [MACRO_CAPTURE(&a,) &b]() {
239+
// CHECK: auto F22b = [MACRO_CAPTURE(&a,) &b]() {
240+
b++;
241+
};
242+
auto F23b = [&a MACRO_CAPTURE(, &b)]() {
243+
// CHECK: auto F23b = [(, &b)]() {
244+
b++;
245+
};
246+
auto F24b = [&a, MACRO_CAPTURE(&b)]() {
247+
// CHECK: auto F24b = [ MACRO_CAPTURE(&b)]() {
248+
b++;
249+
};
250+
}

0 commit comments

Comments
 (0)