-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[FixIt] Improve Source Ranges and Fix-It Hints for Unused Lambda Captures #106445 #117953
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 24 commits
b886394
ccb3952
6b5fff5
46fd602
3e48830
41adf62
420d7d0
ffd9f20
c355082
ff42abb
3ee7a72
5cb9c15
0326d6e
c1326a9
34807aa
cbda68a
f899a90
e41a05a
a830c8a
a7c1d16
82a1667
cd72b63
0f9b9f8
e0cdf27
d5bcc61
9a57baf
fa58b4f
6f38efc
524d0cc
f5ef37c
fb23624
c6fe811
24004a0
56d2d59
22b40e1
c553055
9779723
46ab97b
0fa8171
961c26c
73866bc
8b64a3a
a2c7116
b55c1e9
119e23c
40e6697
be57824
e2adf7c
15dde87
30b5880
4959dab
1ac52d5
9ce2043
e6ec625
d30c84b
675199f
146b98b
746f00e
2687f42
bb74e5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,9 @@ | |
| #include "clang/Sema/SemaOpenMP.h" | ||
| #include "clang/Sema/Template.h" | ||
| #include "llvm/ADT/STLExtras.h" | ||
| #include "clang/Lex/Lexer.h" | ||
| #include "clang/AST/Type.h" | ||
|
|
||
| #include <optional> | ||
| using namespace clang; | ||
| using namespace sema; | ||
|
|
@@ -1163,15 +1166,26 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro, | |
| CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true, | ||
| /*FunctionScopeIndexToStopAtPtr*/ nullptr, | ||
| C->Kind == LCK_StarThis); | ||
| if (!LSI->Captures.empty()) | ||
| LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange; | ||
| continue; | ||
| if (!LSI->Captures.empty()) { // | ||
| SourceManager &SourceMgr = Context.getSourceManager(); | ||
| const LangOptions &LangOpts = Context.getLangOpts(); | ||
| SourceRange TrimmedRange = Lexer::makeFileCharRange( | ||
| CharSourceRange::getTokenRange(C->ExplicitRange), SourceMgr, LangOpts) | ||
| .getAsRange(); | ||
| LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = TrimmedRange; | ||
| } | ||
| continue; // // skip further processing for `this` and `*this` captures. | ||
|
||
| } | ||
|
|
||
| assert(C->Id && "missing identifier for capture"); | ||
| if (!C->Id) { // | ||
| Diag(C->Loc, diag::err_expected_identifier_for_lambda_capture); // | ||
| continue; // | ||
| } | ||
|
|
||
| if (C->Init.isInvalid()) | ||
| continue; | ||
| if (C->Init.isInvalid()) { | ||
| Diag(C->Loc, diag::err_invalid_lambda_capture_initializer_type); // | ||
| continue; // | ||
| } | ||
|
|
||
| ValueDecl *Var = nullptr; | ||
| if (C->Init.isUsable()) { | ||
|
|
@@ -1184,12 +1198,66 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro, | |
| // for e.g., [n{0}] { }; <-- if no <initializer_list> is included. | ||
| // FIXME: we should create the init capture variable and mark it invalid | ||
| // in this case. | ||
| if (C->InitCaptureType.get().isNull()) | ||
| continue; | ||
| // Ensure the initialization is valid before proceeding | ||
|
|
||
|
|
||
| if (!C->InitCaptureType || C->InitCaptureType.get().isNull()) { | ||
| if (!C->Init.isUsable()) { | ||
| Diag(C->Loc, diag::err_invalid_lambda_capture_initializer_type); | ||
| continue; | ||
| } | ||
|
|
||
| if (C->Init.get()->containsUnexpandedParameterPack() && | ||
| !C->InitCaptureType.get()->getAs<PackExpansionType>()) | ||
| DiagnoseUnexpandedParameterPack(C->Init.get(), UPPC_Initializer); | ||
| if (!C->Init.get()) { | ||
| continue; | ||
| } | ||
|
|
||
| ASTContext &Ctx = this->Context; | ||
| QualType DeducedType = C->Init.get()->getType(); | ||
|
|
||
| if (DeducedType.isNull()) { | ||
| continue; | ||
| } | ||
|
|
||
| if (DeducedType->isVoidType()) { | ||
| if (!DeducedType->isDependentType()) { | ||
| C->InitCaptureType = ParsedType::make(Ctx.DependentTy); | ||
| } else { | ||
| Diag(C->Loc, diag::err_invalid_lambda_capture_initializer_type); | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| if (isa<InitListExpr>(C->Init.get())) { | ||
| IdentifierInfo *DummyID = &Ctx.Idents.get("lambda_tmp_var"); | ||
| QualType DummyType = Ctx.UnknownAnyTy; | ||
|
|
||
| auto *TempVarDecl = VarDecl::Create( | ||
| Ctx, nullptr, C->Loc, C->Loc, | ||
| DummyID, DummyType, nullptr, SC_None | ||
| ); | ||
|
|
||
| if (!TempVarDecl) { | ||
| continue; | ||
| } | ||
|
|
||
| DeducedType = deduceVarTypeFromInitializer( | ||
| TempVarDecl, TempVarDecl->getDeclName(), | ||
| TempVarDecl->getType(), nullptr, | ||
| TempVarDecl->getSourceRange(), | ||
| false, C->Init.get() | ||
| ); | ||
|
|
||
| if (DeducedType.isNull()) { | ||
| Diag(C->Loc, diag::err_invalid_lambda_capture_initializer_type); | ||
| C->InitCaptureType = ParsedType::make(Ctx.DependentTy); | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (!DeducedType.isNull()) { | ||
| C->InitCaptureType = ParsedType::make(DeducedType); | ||
| } | ||
| } | ||
|
|
||
| unsigned InitStyle; | ||
| switch (C->InitKind) { | ||
|
|
@@ -1329,7 +1397,13 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro, | |
| tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc); | ||
| } | ||
| if (!LSI->Captures.empty()) | ||
| LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange; | ||
| { | ||
| SourceManager &SourceMgr = Context.getSourceManager(); | ||
| const LangOptions &LangOpts = Context.getLangOpts(); | ||
| SourceRange TrimmedRange = Lexer::makeFileCharRange( | ||
| CharSourceRange::getTokenRange(C->ExplicitRange), SourceMgr, LangOpts).getAsRange(); | ||
| LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = TrimmedRange; | ||
| } | ||
| } | ||
| finishLambdaExplicitCaptures(LSI); | ||
| LSI->ContainsUnexpandedParameterPack |= ContainsUnexpandedParameterPack; | ||
|
|
@@ -2126,32 +2200,35 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, | |
| SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I]; | ||
|
|
||
| // Warn about unused explicit captures. | ||
|
|
||
| bool IsCaptureUsed = true; | ||
| if (!CurContext->isDependentContext() && !IsImplicit && | ||
| !From.isODRUsed()) { | ||
| // Initialized captures that are non-ODR used may not be eliminated. | ||
| // FIXME: Where did the IsGenericLambda here come from? | ||
| bool NonODRUsedInitCapture = | ||
| IsGenericLambda && From.isNonODRUsed() && From.isInitCapture(); | ||
| if (!NonODRUsedInitCapture) { | ||
| bool IsLast = (I + 1) == LSI->NumExplicitCaptures; | ||
| SourceRange FixItRange; | ||
| if (CaptureRange.isValid()) { | ||
| if (!CurHasPreviousCapture && !IsLast) { | ||
| // If there are no captures preceding this capture, remove the | ||
| // following comma. | ||
| FixItRange = SourceRange(CaptureRange.getBegin(), | ||
| getLocForEndOfToken(CaptureRange.getEnd())); | ||
| } else { | ||
| // Otherwise, remove the comma since the last used capture. | ||
| FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc), | ||
| CaptureRange.getEnd()); | ||
| } | ||
| } | ||
|
|
||
| IsCaptureUsed = !DiagnoseUnusedLambdaCapture(FixItRange, From); | ||
| } | ||
| if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) { | ||
| // Handle non-ODR used init captures separately. | ||
| bool NonODRUsedInitCapture = IsGenericLambda && From.isNonODRUsed() && From.isInitCapture(); | ||
|
|
||
| if (!NonODRUsedInitCapture) { | ||
| bool IsLast = (I + 1) == LSI->NumExplicitCaptures; | ||
| SourceRange FixItRange; | ||
|
|
||
| if (CaptureRange.isValid()) { | ||
| if (!CurHasPreviousCapture && !IsLast) { | ||
| // No previous capture and not the last capture: remove current and next comma. | ||
| FixItRange = SourceRange( | ||
| CaptureRange.getBegin(), getLocForEndOfToken(CaptureRange.getEnd())); | ||
| } else if (CurHasPreviousCapture && !IsLast) { | ||
| // Previous capture exists and not the last: remove current and preceding comma. | ||
| FixItRange = SourceRange( | ||
| getLocForEndOfToken(PrevCaptureLoc), CaptureRange.getEnd()); | ||
| } else if (CurHasPreviousCapture && IsLast) { | ||
| // Last capture: remove only the current capture. | ||
| FixItRange = CaptureRange; | ||
| } | ||
| } | ||
|
|
||
| IsCaptureUsed = !DiagnoseUnusedLambdaCapture(FixItRange, From); | ||
| } | ||
| } | ||
|
|
||
| if (CaptureRange.isValid()) { | ||
| CurHasPreviousCapture |= IsCaptureUsed; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,89 +6,121 @@ void test() { | |
| int i = 0; | ||
| int j = 0; | ||
| int k = 0; | ||
| int c = 10; | ||
| int a[c]; | ||
| constexpr int c = 10; | ||
| int a[c]; // Make 'c' constexpr to avoid variable-length array warnings. | ||
|
|
||
| [i,j] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [i,j] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [&i,j] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [j,&i] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [i,j,k] {}; | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [i,j,k] { return i + j; }; | ||
| [i,j] { return i + j; }; | ||
| // CHECK: [i,j] { return i + j; }; | ||
| [i,j,k] { return j + k; }; | ||
| [j,k] { return j + k; }; | ||
| // CHECK: [j,k] { return j + k; }; | ||
| [i,j,k] { return i + k; }; | ||
| [i,k] { return i + k; }; | ||
| // CHECK: [i,k] { return i + k; }; | ||
| [i,j,k] { return i + j + k; }; | ||
| // CHECK: [i,j,k] { return i + j + k; }; | ||
| [&,i] { return k; }; | ||
| [&] { return k; }; | ||
| // CHECK: [&] { return k; }; | ||
| [=,&i] { return k; }; | ||
| [=] { return k; }; | ||
| // CHECK: [=] { return k; }; | ||
| [=,&i,&j] { return j; }; | ||
| [=,&j] { return j; }; | ||
| // CHECK: [=,&j] { return j; }; | ||
| [=,&i,&j] { return i; }; | ||
| [=,&i] { return i; }; | ||
| // CHECK: [=,&i] { return i; }; | ||
| [z = i] {}; | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [i,z = i] { return z; }; | ||
| [z = i] { return z; }; | ||
| // CHECK: [z = i] { return z; }; | ||
| [z = i,i] { return z; }; | ||
| [z = i] { return z; }; | ||
| // CHECK: [z = i] { return z; }; | ||
| [&a] {}; | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [i,&a] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [&a,i] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
|
||
|
|
||
| #define I_MACRO() i | ||
| #define I_REF_MACRO() &i | ||
| [I_MACRO()] {}; | ||
| #define I_MACRO() i | ||
| #define I_REF_MACRO() &i | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [I_MACRO(),j] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [j,I_MACRO()] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [I_REF_MACRO(),j] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [j,I_REF_MACRO()] { return j; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
|
|
||
| int n = 0; | ||
| [z = (n = i),j] {}; | ||
| [z = (n = i)] {}; | ||
| // CHECK: [z = (n = i)] {}; | ||
| [j,z = (n = i)] {}; | ||
| [z = (n = i)] {}; | ||
| // CHECK: [z = (n = i)] {}; | ||
|
|
||
| // New Edge Cases | ||
|
|
||
| // Test 1: Leading and trailing whitespace | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [j,k] { return j + k; }; | ||
| // CHECK: [j,k] { return j + k; }; | ||
|
|
||
| // Test 2: Single unused capture | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
|
|
||
| // Test 3: Multiple commas | ||
| [j] { return j; }; | ||
| // CHECK: [j] { return j; }; | ||
| [k] { return k; }; | ||
| // CHECK: [k] { return k; }; | ||
|
|
||
| // Test 4: Mixed captures | ||
| [&i] { return i; }; | ||
| // CHECK: [&i] { return i; }; | ||
| [&] {}; | ||
| // CHECK: [&] {}; | ||
|
|
||
| // Test 5: Capture with comments | ||
| [/*capture*/ j] { return j; }; | ||
| // CHECK: [/*capture*/ j] { return j; }; | ||
| } | ||
|
|
||
| class ThisTest { | ||
| void test() { | ||
| int i = 0; | ||
|
|
||
| [this] {}; | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [i,this] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [this,i] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [*this] {}; | ||
| [] {}; | ||
| // CHECK: [] {}; | ||
| [*this,i] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [i,*this] { return i; }; | ||
| [i] { return i; }; | ||
| // CHECK: [i] { return i; }; | ||
| [*this] { return this; }; | ||
| // CHECK: [*this] { return this; }; | ||
| [*this,i] { return this; }; | ||
| [*this] { return this; }; | ||
| // CHECK: [*this] { return this; }; | ||
| [i,*this] { return this; }; | ||
| [*this] { return this; }; | ||
| // CHECK: [*this] { return this; }; | ||
| } | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you've last the code to emit these :-O