https://github.com/ojhunt created
https://github.com/llvm/llvm-project/pull/141148
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.
>From 0db205bb8f6925dd725885395f58a688000c7b8b Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Thu, 22 May 2025 02:38:26 -0700
Subject: [PATCH] [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.
---
clang/include/clang/Sema/Sema.h | 6 +
clang/lib/Sema/Sema.cpp | 15 ++
clang/lib/Sema/SemaLambda.cpp | 26 +-
...-unused-lambda-capture-trailing-tokens.cpp | 250 ++++++++++++++++++
4 files changed, 291 insertions(+), 6 deletions(-)
create mode 100644
clang/test/FixIt/fixit-unused-lambda-capture-trailing-tokens.cpp
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a994b845e11fc..f83047d63adab 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -972,6 +972,12 @@ class Sema final : public SemaBase {
/// Calls \c Lexer::getLocForEndOfToken()
SourceLocation getLocForEndOfToken(SourceLocation Loc, unsigned Offset = 0);
+ /// Calls \c Lexer::findNextToken() to find the next token, and if the
+ /// locations of both ends of the token can be resolved it return that
+ /// range; Otherwise it returns an invalid SourceRange.
+ SourceRange getRangeForNextToken(SourceLocation Loc,
+ bool IncludeComments = false);
+
/// Retrieve the module loader associated with the preprocessor.
ModuleLoader &getModuleLoader() const;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 1901d19b14dfc..9f706579b1b79 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -84,6 +84,21 @@ SourceLocation Sema::getLocForEndOfToken(SourceLocation Loc,
unsigned Offset) {
return Lexer::getLocForEndOfToken(Loc, Offset, SourceMgr, LangOpts);
}
+SourceRange Sema::getRangeForNextToken(SourceLocation Loc,
+ bool IncludeComments) {
+ if (!Loc.isValid())
+ return SourceRange();
+ std::optional<Token> NextToken =
+ Lexer::findNextToken(Loc, SourceMgr, LangOpts, IncludeComments);
+ if (!NextToken)
+ return SourceRange();
+ SourceLocation TokenStart = NextToken->getLocation();
+ SourceLocation TokenEnd = NextToken->getLastLoc();
+ if (!TokenStart.isValid() || !TokenEnd.isValid())
+ return SourceRange();
+ return SourceRange(TokenStart, TokenEnd);
+}
+
ModuleLoader &Sema::getModuleLoader() const { return PP.getModuleLoader(); }
DarwinSDKInfo *
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index aad16290422f5..d55dbf82ffdc0 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -2164,15 +2164,29 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation
StartLoc, SourceLocation EndLoc,
bool IsLast = (I + 1) == LSI->NumExplicitCaptures;
SourceRange FixItRange;
if (CaptureRange.isValid()) {
+ auto GetTrailingEndLocation = [&](SourceLocation StartPoint) {
+ SourceRange NextToken =
+ getRangeForNextToken(StartPoint, /*IncludeComments=*/true);
+ if (!NextToken.isValid())
+ return SourceLocation();
+ // Return the last location preceding the next token
+ return NextToken.getBegin().getLocWithOffset(-1);
+ };
if (!CurHasPreviousCapture && !IsLast) {
// If there are no captures preceding this capture, remove the
- // following comma.
- FixItRange = SourceRange(CaptureRange.getBegin(),
-
getLocForEndOfToken(CaptureRange.getEnd()));
+ // trailing comma and anything up to the next token
+ SourceRange CommaRange =
+ getRangeForNextToken(CaptureRange.getEnd());
+ SourceLocation FixItEnd =
+ GetTrailingEndLocation(CommaRange.getBegin());
+ FixItRange = SourceRange(CaptureRange.getBegin(), FixItEnd);
} else {
- // Otherwise, remove the comma since the last used capture.
- FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc),
- CaptureRange.getEnd());
+ // Otherwise, remove the comma since the last used capture, and
+ // anything up to the next token
+ SourceLocation FixItStart = getLocForEndOfToken(PrevCaptureLoc);
+ SourceLocation FixItEnd =
+ GetTrailingEndLocation(CaptureRange.getEnd());
+ FixItRange = SourceRange(FixItStart, FixItEnd);
}
}
diff --git a/clang/test/FixIt/fixit-unused-lambda-capture-trailing-tokens.cpp
b/clang/test/FixIt/fixit-unused-lambda-capture-trailing-tokens.cpp
new file mode 100644
index 0000000000000..e2c2cdb6a2d92
--- /dev/null
+++ b/clang/test/FixIt/fixit-unused-lambda-capture-trailing-tokens.cpp
@@ -0,0 +1,250 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -Wunused-lambda-capture -Wno-unused-value -std=c++1z
-fixit %t
+// RUN: grep -v CHECK %t | FileCheck %s
+
+
+#define MACRO_CAPTURE(...) __VA_ARGS__
+int main() {
+ int a = 0, b = 0, c = 0;
+ auto F0 = [a, &b]() mutable {
+ // CHECK: auto F0 = [a]()
+ a++;
+ };
+
+ auto F1 = [&a, &b]() {
+ // CHECK: auto F1 = []() {
+ };
+
+ auto F2 = [&a, b]() {
+ // CHECK: auto F2 = []() {
+ };
+
+ auto F3 = [&a,
+ &b]() {
+ // CHECK: auto F3 = []() {
+ };
+
+ auto F4 = [&a
+ , &b]() {
+ // CHECK: auto F4 = []() {
+ };
+ auto F5 = [&a ,&b]() {
+ // CHECK: auto F5 = []() {
+ };
+
+ auto F0a = [a, &b]() mutable {
+ // CHECK: auto F0a = [a]() mutable {
+ a++;
+ };
+
+ auto F1a = [&a, &b]() {
+ // CHECK: auto F1a = [&a]() {
+ a++;
+ };
+
+ auto F2a = [&a, b]() {
+ // CHECK: auto F2a = [&a]() {
+ a++;
+ };
+
+ auto F3a = [&a,
+ &b]() {
+ // CHECK: auto F3a = [&a]() {
+ a++;
+ };
+
+ auto F4a = [&a
+ , &b]() {
+ // CHECK: auto F4a = [&a]() {
+ a++;
+ };
+
+ auto F5a = [&a ,&b]() {
+ // CHECK: auto F5a = [&a]() {
+ a++;
+ };
+ auto F0b = [a, &b]() mutable {
+ // CHECK: auto F0b = [ &b]() mutable
+ b++;
+ };
+
+ auto F1b = [&a, &b]() {
+ // CHECK: auto F1b = [ &b]() {
+ b++;
+ };
+
+ auto F2b = [&a, b]() mutable {
+ // CHECK: auto F2b = [ b]() mutable {
+ b++;
+ };
+
+ auto F3b = [&a,
+ &b]() {
+ // CHECK: auto F3b = [ &b]() {
+ b++;
+ };
+
+ auto F4b = [&a
+ , &b]() {
+ // CHECK: auto F4b = [ &b]() {
+ b++;
+ };
+ auto F5b = [&a ,&b]() {
+ // CHECK: auto F5b = [&b]() {
+ b++;
+ };
+
+ auto F6 = [&a, &b, &c]() {
+ // CHECK: auto F6 = [&a, &b]() {
+ a++;
+ b++;
+ };
+ auto F7 = [&a, &b, &c]() {
+ // CHECK: auto F7 = [&a, &c]() {
+ a++;
+ c++;
+ };
+ auto F8 = [&a, &b, &c]() {
+ // CHECK: auto F8 = [ &b, &c]() {
+ b++;
+ c++;
+ };
+ auto F9 = [&a, &b , &c]() {
+ // CHECK: auto F9 = [&a , &c]() {
+ a++;
+ c++;
+ };
+ auto F10 = [&a,
+ &b, &c]() {
+ // CHECK: auto F10 = [&a, &c]() {
+ a++;
+ c++;
+ };
+ auto F11 = [&a, &b ,
+ &c]() {
+ // CHECK: auto F11 = [&a ,
+ // CHECK-NEXT: &c]() {
+ a++;
+ c++;
+ };
+ auto F12 = [a = 0, b ,
+ c]() mutable {
+ // CHECK: auto F12 = [ b ,
+ // CHECK-NEXT: c]() mutable {
+ b++;
+ c++;
+ };
+ auto F13 = [a, b = 0 ,
+ c]() mutable {
+ // CHECK: auto F13 = [a ,
+ // CHECK-NEXT: c]() mutable {
+ a++;
+ c++;
+ };
+ auto F14 = [a, b ,
+ c
+ = 0]() mutable {
+ // CHECK: auto F14 = [a, b]() mutable {
+ a++;
+ b++;
+ };
+
+ // We want to remove everything including the comment
+ // as well as the comma following the capture of `a`
+ auto F15 = [&a /* comment */, &b]() {
+ // CHECK: auto F15 = [ &b]() {
+ b++;
+ };
+
+ // The comment preceding the first capture remains. This is more
+ // by design of the fixit logic than anything else, but we should
+ // consider the preceding comment might actually be a comment for
+ // the entire capture set, so maybe we do want it to hang around
+ auto F16 = [/* comment */ &a , &b]() {
+ // CHECK: auto F16 = [/* comment */ &b]() {
+ b++;
+ };
+
+ auto F16b = [&a , /* comment */ &b]() {
+ // CHECK: auto F16b = [ /* comment */ &b]() {
+ b++;
+ };
+
+ auto F17 = [&a /* comment */, &b]() {
+ // CHECK: auto F17 = [&a]() {
+ a++;
+ };
+
+ auto F18 = [&a , /* comment */ &b]() {
+ // CHECK: auto F18 = [&a]() {
+ a++;
+ };
+
+ auto F19 = [&a /* comment */, &b /* comment */]() {
+ // CHECK: auto F19 = [&a /* comment */]() {
+ a++;
+ };
+
+ auto F20 = [MACRO_CAPTURE(&a, &b)]() {
+ // CHECK: auto F20 = [MACRO_CAPTURE(&a, &b)]() {
+ };
+
+ auto F21 = [MACRO_CAPTURE(&a), &b]() {
+ // CHECK: auto F21 = []() {
+ };
+
+ auto F22 = [MACRO_CAPTURE(&a,) &b]() {
+ // CHECK: auto F22 = [MACRO_CAPTURE(&a,) &b]() {
+ };
+ auto F23 = [&a MACRO_CAPTURE(, &b)]() {
+ // CHECK: auto F23 = []() {
+ };
+ auto F24 = [&a, MACRO_CAPTURE(&b)]() {
+ // CHECK: auto F24 = []() {
+ };
+
+ auto F20a = [MACRO_CAPTURE(&a, &b)]() {
+ // CHECK: auto F20a = [MACRO_CAPTURE(&a, &b)]() {
+ a++;
+ };
+
+ auto F21a = [MACRO_CAPTURE(&a), &b]() {
+ // CHECK: auto F21a = [MACRO_CAPTURE(&a)]() {
+ a++;
+ };
+
+ auto F22a = [MACRO_CAPTURE(&a,) &b]() {
+ // Cauto F22a = [MACRO_CAPTURE(&a,) &b]() {
+ a++;
+ };
+ auto F23a = [&a MACRO_CAPTURE(, &b)]() {
+ // CHECK: auto F23a = [&a]() {
+ a++;
+ };
+ auto F24a = [&a, MACRO_CAPTURE(&b)]() {
+ // CHECK: auto F24a = [&a]() {
+ a++;
+ };
+ auto F20b = [MACRO_CAPTURE(&a, &b)]() {
+ // CHECK: auto F20b = [MACRO_CAPTURE(&a, &b)]() {
+ b++;
+ };
+
+ auto F21b = [MACRO_CAPTURE(&a), &b]() {
+ // CHECK: auto F21b = [ &b]() {
+ b++;
+ };
+
+ auto F22b = [MACRO_CAPTURE(&a,) &b]() {
+ // CHECK: auto F22b = [MACRO_CAPTURE(&a,) &b]() {
+ b++;
+ };
+ auto F23b = [&a MACRO_CAPTURE(, &b)]() {
+ // CHECK: auto F23b = [(, &b)]() {
+ b++;
+ };
+ auto F24b = [&a, MACRO_CAPTURE(&b)]() {
+ // CHECK: auto F24b = [ MACRO_CAPTURE(&b)]() {
+ b++;
+ };
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
