From 832f49b90a4907a0bc2b34b688c32f3c9613aab0 Mon Sep 17 00:00:00 2001 From: Alexander Shaposhnikov Date: Mon, 16 Jul 2018 07:23:47 +0000 Subject: [PATCH] [Sema] Add fixit for unused lambda captures This diff adds a fixit to suggest removing unused lambda captures in the appropriate diagnostic. Patch by Andrew Comminos! Test plan: make check-all Differential revision: https://reviews.llvm.org/D48845 llvm-svn: 337148 --- clang/include/clang/Sema/DeclSpec.h | 13 +++- clang/include/clang/Sema/ScopeInfo.h | 3 + clang/include/clang/Sema/Sema.h | 6 +- clang/lib/Parse/ParseExprCXX.cpp | 6 +- clang/lib/Sema/SemaLambda.cpp | 48 ++++++++++-- clang/test/FixIt/fixit-unused-lambda-capture.cpp | 94 ++++++++++++++++++++++++ 6 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 clang/test/FixIt/fixit-unused-lambda-capture.cpp diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h index 02afb093..40e5269 100644 --- a/clang/include/clang/Sema/DeclSpec.h +++ b/clang/include/clang/Sema/DeclSpec.h @@ -2540,12 +2540,16 @@ struct LambdaIntroducer { LambdaCaptureInitKind InitKind; ExprResult Init; ParsedType InitCaptureType; + SourceRange ExplicitRange; + LambdaCapture(LambdaCaptureKind Kind, SourceLocation Loc, IdentifierInfo *Id, SourceLocation EllipsisLoc, LambdaCaptureInitKind InitKind, ExprResult Init, - ParsedType InitCaptureType) + ParsedType InitCaptureType, + SourceRange ExplicitRange) : Kind(Kind), Loc(Loc), Id(Id), EllipsisLoc(EllipsisLoc), - InitKind(InitKind), Init(Init), InitCaptureType(InitCaptureType) {} + InitKind(InitKind), Init(Init), InitCaptureType(InitCaptureType), + ExplicitRange(ExplicitRange) {} }; SourceRange Range; @@ -2563,9 +2567,10 @@ struct LambdaIntroducer { SourceLocation EllipsisLoc, LambdaCaptureInitKind InitKind, ExprResult Init, - ParsedType InitCaptureType) { + ParsedType InitCaptureType, + SourceRange ExplicitRange) { Captures.push_back(LambdaCapture(Kind, Loc, Id, EllipsisLoc, InitKind, Init, - InitCaptureType)); + InitCaptureType, ExplicitRange)); } }; diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index fa1b103..1074074 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -829,6 +829,9 @@ public: /// if the enclosing full-expression is instantiation dependent). llvm::SmallSet NonODRUsedCapturingExprs; + /// A map of explicit capture indices to their introducer source ranges. + llvm::DenseMap ExplicitCaptureRanges; + /// Contains all of the variables defined in this lambda that shadow variables /// that were defined in parent contexts. Used to avoid warnings when the /// shadowed variables are uncaptured by this lambda. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index ec60bb0..2425dff 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5602,8 +5602,10 @@ public: /// Does copying/destroying the captured variable have side effects? bool CaptureHasSideEffects(const sema::Capture &From); - /// Diagnose if an explicit lambda capture is unused. - void DiagnoseUnusedLambdaCapture(const sema::Capture &From); + /// Diagnose if an explicit lambda capture is unused. Returns true if a + /// diagnostic is emitted. + bool DiagnoseUnusedLambdaCapture(SourceRange CaptureRange, + const sema::Capture &From); /// Complete a lambda-expression having processed and attached the /// lambda body. diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp index 6470697..26e7599 100644 --- a/clang/lib/Parse/ParseExprCXX.cpp +++ b/clang/lib/Parse/ParseExprCXX.cpp @@ -808,6 +808,7 @@ Optional Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro, IdentifierInfo *Id = nullptr; SourceLocation EllipsisLoc; ExprResult Init; + SourceLocation LocStart = Tok.getLocation(); if (Tok.is(tok::star)) { Loc = ConsumeToken(); @@ -981,8 +982,11 @@ Optional Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro, Loc, Kind == LCK_ByRef, Id, InitKind, InitExpr); Init = InitExpr; } + + SourceLocation LocEnd = PrevTokLocation; + Intro.addCapture(Kind, Loc, Id, EllipsisLoc, InitKind, Init, - InitCaptureType); + InitCaptureType, SourceRange(LocStart, LocEnd)); } T.consumeClose(); diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index 9e53beb4..0b768cc 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -993,6 +993,8 @@ void Sema::ActOnStartOfLambdaDefinition(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; } @@ -1139,6 +1141,8 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, TryCapture_ExplicitByVal; tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc); } + if (!LSI->Captures.empty()) + LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange; } finishLambdaExplicitCaptures(LSI); @@ -1478,12 +1482,13 @@ bool Sema::CaptureHasSideEffects(const Capture &From) { return false; } -void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) { +bool Sema::DiagnoseUnusedLambdaCapture(SourceRange CaptureRange, + const Capture &From) { if (CaptureHasSideEffects(From)) - return; + return false; if (From.isVLATypeCapture()) - return; + return false; auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture); if (From.isThisCapture()) @@ -1491,6 +1496,8 @@ void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) { else diag << From.getVariable(); diag << From.isNonODRUsed(); + diag << FixItHint::CreateRemoval(CaptureRange); + return true; } ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, @@ -1532,18 +1539,49 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, // Translate captures. auto CurField = Class->field_begin(); + // True if the current capture has a used capture or default before it. + bool CurHasPreviousCapture = CaptureDefault != LCD_None; + SourceLocation PrevCaptureLoc = CurHasPreviousCapture ? + CaptureDefaultLoc : IntroducerRange.getBegin(); + for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) { const Capture &From = LSI->Captures[I]; + assert(!From.isBlockCapture() && "Cannot capture __block variables"); bool IsImplicit = I >= LSI->NumExplicitCaptures; + // Use source ranges of explicit captures for fixits where available. + 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. bool NonODRUsedInitCapture = IsGenericLambda && From.isNonODRUsed() && From.getInitExpr(); - if (!NonODRUsedInitCapture) - DiagnoseUnusedLambdaCapture(From); + 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 (CaptureRange.isValid()) { + CurHasPreviousCapture |= IsCaptureUsed; + PrevCaptureLoc = CaptureRange.getEnd(); } // Handle 'this' capture. diff --git a/clang/test/FixIt/fixit-unused-lambda-capture.cpp b/clang/test/FixIt/fixit-unused-lambda-capture.cpp new file mode 100644 index 0000000..c312009 --- /dev/null +++ b/clang/test/FixIt/fixit-unused-lambda-capture.cpp @@ -0,0 +1,94 @@ +// RUN: cp %s %t +// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -Wno-unused-value -std=c++1z -fixit %t +// RUN: grep -v CHECK %t | FileCheck %s + +void test() { + int i = 0; + int j = 0; + int k = 0; + int c = 10; + int a[c]; + + [i,j] { return i; }; + // CHECK: [i] { return i; }; + [i,j] { return j; }; + // CHECK: [j] { return j; }; + [&i,j] { return j; }; + // CHECK: [j] { return j; }; + [j,&i] { return j; }; + // CHECK: [j] { return j; }; + [i,j,k] {}; + // CHECK: [] {}; + [i,j,k] { return i + j; }; + // CHECK: [i,j] { return i + j; }; + [i,j,k] { return j + k; }; + // CHECK: [j,k] { return j + k; }; + [i,j,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; }; + // CHECK: [&] { return k; }; + [=,&i] { return k; }; + // CHECK: [=] { return k; }; + [=,&i,&j] { return j; }; + // CHECK: [=,&j] { return j; }; + [=,&i,&j] { return i; }; + // CHECK: [=,&i] { return i; }; + [z = i] {}; + // CHECK: [] {}; + [i,z = i] { return z; }; + // CHECK: [z = i] { return z; }; + [z = i,i] { return z; }; + // CHECK: [z = i] { return z; }; + [&a] {}; + // CHECK: [] {}; + [i,&a] { return i; }; + // CHECK: [i] { return i; }; + [&a,i] { return i; }; + // CHECK: [i] { return i; }; + + #define I_MACRO() i + #define I_REF_MACRO() &i + [I_MACRO()] {}; + // CHECK: [] {}; + [I_MACRO(),j] { return j; }; + // CHECK: [j] { return j; }; + [j,I_MACRO()] { return j; }; + // CHECK: [j] { return j; }; + [I_REF_MACRO(),j] { return j; }; + // CHECK: [j] { return j; }; + [j,I_REF_MACRO()] { return j; }; + // CHECK: [j] { return j; }; + + int n = 0; + [z = (n = i),j] {}; + // CHECK: [z = (n = i)] {}; + [j,z = (n = i)] {}; + // CHECK: [z = (n = i)] {}; +} + +class ThisTest { + void test() { + int i = 0; + + [this] {}; + // CHECK: [] {}; + [i,this] { return i; }; + // CHECK: [i] { return i; }; + [this,i] { return i; }; + // CHECK: [i] { return i; }; + [*this] {}; + // CHECK: [] {}; + [*this,i] { return i; }; + // CHECK: [i] { return i; }; + [i,*this] { return i; }; + // CHECK: [i] { return i; }; + [*this] { return this; }; + // CHECK: [*this] { return this; }; + [*this,i] { return this; }; + // CHECK: [*this] { return this; }; + [i,*this] { return this; }; + // CHECK: [*this] { return this; }; + } +}; -- 2.7.4