[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 153968.
acomminos retitled this revision from "[Sema] Add fixit for 
-Wno-unused-lambda-capture" to "[Sema] Add fixit for unused lambda captures".
acomminos edited the summary of this revision.
acomminos changed the visibility from "Custom Policy" to "Public (No Login 
Required)".
acomminos added a subscriber: alexshap.
acomminos added a comment.
Herald added a subscriber: cfe-commits.

Added tests, add logic for removing a comma forward for beginning edge case.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,31 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -std=c++1z -fixit %t
+// RUN: grep -v CHECK %t | FileCheck %s
+
+void test() {
+  int i = 0;
+  int j = 0;
+  int k = 0;
+  [i,j] { return i; };
+  // CHECK: [i] { return i; };
+  [i,j] { 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; };
+}
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1478,7 +1478,8 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
 return;
 
@@ -1491,6 +1492,7 @@
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,19 +1534,40 @@
 
 // Translate captures.
 auto CurField = Class->field_begin();
+// True if the current capture has an initializer or default before it.
+bool CurHasPreviousInitializer = CaptureDefault != LCD_None;
+SourceLocation PrevCaptureLoc = CurHasPreviousInitializer ?
+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;
 
   // 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) {
+  // Delete either the preceding or next comma in the explicit capture
+  // list, depending on whether or not elements follow.
+  SourceRange FixItRange;
+  bool IsLast = I + 1 == LSI->NumExplicitCaptures;
+  if (!CurHasPreviousInitializer && !IsLast) {
+FixItRange = SourceRange(From.getLocation(),
+ getLocForEndOfToken(From.getLocation()));
+  } else {
+FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc),
+ From.getLocation());
+  }
+
+  DiagnoseUnusedLambdaCapture(FixItRange, From);
+  IsCaptureUsed = false;
+}
   }
+  CurHasPreviousInitializer |= IsCaptureUsed;
 
   // Handle 'this' capture.
   if (From.isThisCapture()) {
@@ -1574,6 +1597,8 @@
 Init = InitResult.get();
   }
   CaptureInits.push_back(Init);
+
+  PrevCaptureLoc = From.getLocation();
 }
 
 // C++11 [expr.prim.lambda]p6:
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -5604,7 +5604,8 @@
   bool CaptureHasSideEffects(const sema::Capture &From);
 
   /// Diagnose if an explicit lambda capture is unused.
-  void DiagnoseUnusedLambdaCapture(const sema::Capture &From);
+  void DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+ 

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos planned changes to this revision.
acomminos added a comment.

Ah yes, thanks for pointing this out. Some additional logic is going to be 
necessary to handle capture initializers correctly- I'll look into exposing 
full source ranges in LambdaCapture to make this more consistent across capture 
types.


Repository:
  rC Clang

https://reviews.llvm.org/D48845



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 154307.
acomminos added a comment.

Handle initialization expressions and dereferenced `this` in lambda captures.

An alternative to handling various kinds of explicit captures would be 
propagating the source range for each lambda capture from the parser to each 
sema::Capture. This would only be applicable to explicit captures; is this 
preferable?


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,52 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -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,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,j] { return z; };
+  // CHECK: [z = i] { return z; };
+  [&a] {};
+  // CHECK: [] {};
+}
+
+class ThisTest {
+  void test() {
+int i = 0;
+[this] {};
+// CHECK: [] {};
+[i,this] { return this; };
+// CHECK: [this] { return this; };
+[*this] {};
+// CHECK: [] {};
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1478,7 +1478,8 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
 return;
 
@@ -1491,6 +1492,7 @@
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,19 +1534,51 @@
 
 // Translate captures.
 auto CurField = Class->field_begin();
+// True if the current capture has an initializer or default before it.
+bool CurHasPreviousInitializer = CaptureDefault != LCD_None;
+SourceLocation PrevCaptureLoc = CurHasPreviousInitializer ?
+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;
 
+  // Find the end of the explicit capture for use in fixits.
+  SourceLocation EndLoc;
+  if (From.isThisCapture() && From.isCopyCapture()) {
+// Skip dereference token in *this.
+EndLoc = getLocForEndOfToken(From.getLocation());
+  } else if (!From.isVLATypeCapture() && From.getInitExpr()) {
+// For initialized captures, use the end of the expression.
+EndLoc = From.getInitExpr()->getLocEnd();
+  } else {
+// Otherwise, use the location of the identifier token.
+EndLoc = From.getLocation();
+  }
+
   // 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) {
+  // Delete either the preceding or next comma in the explicit capture
+  // list, depending on whether or not elements follow.
+  SourceRange FixItRange;
+  bool IsLast = I + 1 == LSI->NumExplicitCaptures;
+  if (!CurHasPreviousInitializer && !IsLast) {
+FixItRange = SourceRange(From.getLocation(), getLocForEndOfToken(EndLoc));
+  } else {
+FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc), EndLoc);
+  }
+
+  DiagnoseUnusedLambdaCapture(FixItRange, From);
+  IsCaptureUsed = false;
+}
   }
+  CurHasPrevio

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 154313.
acomminos added a comment.

Add additional tests to ensure that explicit capture ranges are predicted 
correctly.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,58 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -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,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,j] { return z; };
+  // CHECK: [z = i] { return z; };
+  [&a] {};
+  // CHECK: [] {};
+}
+
+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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1478,7 +1478,8 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
 return;
 
@@ -1491,6 +1492,7 @@
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,19 +1534,51 @@
 
 // Translate captures.
 auto CurField = Class->field_begin();
+// True if the current capture has an initializer or default before it.
+bool CurHasPreviousInitializer = CaptureDefault != LCD_None;
+SourceLocation PrevCaptureLoc = CurHasPreviousInitializer ?
+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;
 
+  // Find the end of the explicit capture for use in fixits.
+  SourceLocation EndLoc;
+  if (From.isThisCapture() && From.isCopyCapture()) {
+// Skip dereference token in *this.
+EndLoc = getLocForEndOfToken(From.getLocation());
+  } else if (!From.isVLATypeCapture() && From.getInitExpr()) {
+// For initialized captures, use the end of the expression.
+EndLoc = From.getInitExpr()->getLocEnd();
+  } else {
+// Otherwise, use the location of the identifier token.
+EndLoc = From.getLocation();
+  }
+
   // 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) {
+  // Include either the previous, next, or no comma to produce
+  // individually valid fixits, depending on the capture position.
+  SourceRange FixItRange;
+  bool IsLast = I + 1 == LSI->NumExplicitCaptures;
+  if (!CurHasPreviousInitializer && !IsLast) {
+FixItRange = SourceRange(From.getLocation(), getLocForEndOfToken(EndLoc));
+  } else {
+FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc), EndLoc);
+  }
+
+  DiagnoseUnusedLambdaCapture(FixItRange, From);
+  IsCaptureUsed = false;
+}
   }
+  CurHasPreviousInitializer |= IsCaptureUsed;
 
   // Ha

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 155246.
acomminos added a comment.

Thanks for the feedback! This diff switches to using a source range for 
captures provided by the parser, which is more accurate, future-proof, and 
correctly handles macros.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,71 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -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,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,j] { return z; };
+  // CHECK: [z = i] { return z; };
+  [&a] {};
+  // CHECK: [] {};
+
+  #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; };
+}
+
+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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -824,6 +824,7 @@
   LSI->addCapture(Var, /*isBlock*/false, Var->getType()->isReferenceType(),
   /*isNested*/false, Var->getLocation(), SourceLocation(),
   Var->getType(), Var->getInit());
+
   return Field;
 }
 
@@ -954,6 +955,9 @@
 = Intro.Default == LCD_None? Intro.Range.getBegin() : Intro.DefaultLoc;
   for (auto C = Intro.Captures.begin(), E = Intro.Captures.end(); C != E;
PrevCaptureLoc = C->Loc, ++C) {
+LSI->ExplicitCaptureRanges[C - Intro.Captures.begin()] =
+SourceRange(C->LocStart, C->LocEnd);
+
 if (C->Kind == LCK_This || C->Kind == LCK_StarThis) {
   if (C->Kind == LCK_StarThis) 
 Diag(C->Loc, !getLangOpts().CPlusPlus17
@@ -1478,7 +1482,8 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
 return;
 
@@ -1491,6 +1496,7 @@
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,19 +1538,47 @@
 
 // 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];
+  if (CaptureRange.isInvalid()) {
+CaptureRange = SourceRange(From.getLocation());
+  }
+
   // 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.

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 155428.
acomminos marked 2 inline comments as done.
acomminos added a comment.

Thanks! Updated to be more explicit about location names, add more tests for 
VLA and *this captures, and fix an issue with VLA range captures invalidating 
the capture range tracking.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,88 @@
+// 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; };
+}
+
+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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -993,6 +993,8 @@
   CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true,
   /*FunctionScopeIndexToStopAtPtr*/ nullptr,
   C->Kind == LCK_StarThis);
+  LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] =
+  SourceRange(C->ExplicitLocStart, C->ExplicitLocEnd);
   continue;
 }
 
@@ -1139,6 +1141,8 @@
TryCapture_ExplicitByVal;
   tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
 }
+LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] =
+SourceRange(C->ExplicitLocStart, C->ExplicitLocEnd);
   }
   finishLambdaExplicitCaptures(LSI);
 
@@ -1478,7 +1482,8 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
 return;
 
@@ -1491,6 +1496,7 @@
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,18 +1538,50 @@
 
 // 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

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 155430.
acomminos marked 2 inline comments as done.
acomminos added a comment.

Elide braces in single-line conditional.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,88 @@
+// 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; };
+}
+
+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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -993,6 +993,8 @@
   CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true,
   /*FunctionScopeIndexToStopAtPtr*/ nullptr,
   C->Kind == LCK_StarThis);
+  LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] =
+  SourceRange(C->ExplicitLocStart, C->ExplicitLocEnd);
   continue;
 }
 
@@ -1139,6 +1141,8 @@
TryCapture_ExplicitByVal;
   tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
 }
+LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] =
+SourceRange(C->ExplicitLocStart, C->ExplicitLocEnd);
   }
   finishLambdaExplicitCaptures(LSI);
 
@@ -1478,7 +1482,8 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+   const Capture &From) {
   if (CaptureHasSideEffects(From))
 return;
 
@@ -1491,6 +1496,7 @@
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,20 +1538,51 @@
 
 // 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 fix

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 155446.
acomminos marked 2 inline comments as done.
acomminos added a comment.

Add test for stateful initializer expressions not being removed, propagate 
whether or not a diagnostic actually get emitted.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ 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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -993,6 +993,8 @@
   CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true,
   /*FunctionScopeIndexToStopAtPtr*/ nullptr,
   C->Kind == LCK_StarThis);
+  LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] =
+  SourceRange(C->ExplicitLocStart, C->ExplicitLocEnd);
   continue;
 }
 
@@ -1139,6 +1141,8 @@
TryCapture_ExplicitByVal;
   tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
 }
+LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] =
+SourceRange(C->ExplicitLocStart, C->ExplicitLocEnd);
   }
   finishLambdaExplicitCaptures(LSI);
 
@@ -1478,19 +1482,22 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+bool Sema::DiagnoseUnusedLambdaCapture(const 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())
 diag << "'this'";
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
+  return true;
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,18 +1539,49 @@
 
 // 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 Pr

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos marked an inline comment as done.
acomminos added inline comments.



Comment at: include/clang/Sema/DeclSpec.h:2552-2553
 ParsedType InitCaptureType;
+SourceLocation LocStart;
+SourceLocation LocEnd;
+

aaron.ballman wrote:
> aaron.ballman wrote:
> > How does `LocStart` relate to the existing source location `Loc`? I think 
> > this should have a more descriptive name of what location is involved.
> Now that I think about this more, I wonder if this is better expressed as 
> `SourceRange CaptureRange;` given that there's always a start and end and 
> they should never be equal?
Sure, makes sense to me. Many other classes with attached range data appear to 
use explicit start/end locations, but in this case I think it's fine.


Repository:
  rC Clang

https://reviews.llvm.org/D48845



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 155472.
acomminos marked an inline comment as done.
acomminos added a comment.

Use source ranges instead of a pair of source locations for explicit lambda 
captures.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ 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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -993,6 +993,7 @@
   CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true,
   /*FunctionScopeIndexToStopAtPtr*/ nullptr,
   C->Kind == LCK_StarThis);
+  LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   continue;
 }
 
@@ -1139,6 +1140,7 @@
TryCapture_ExplicitByVal;
   tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
 }
+LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   }
   finishLambdaExplicitCaptures(LSI);
 
@@ -1478,19 +1480,22 @@
   return false;
 }
 
-void Sema::DiagnoseUnusedLambdaCapture(const Capture &From) {
+bool Sema::DiagnoseUnusedLambdaCapture(const 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())
 diag << "'this'";
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
+  return true;
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,18 +1537,49 @@
 
 // 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 = L

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

[Andrew Comminos via Phabricator via cfe-commits]

acomminos updated this revision to Diff 155616.
acomminos added a comment.

Remove `const` qualifier for SourceRange.


Repository:
  rC Clang

https://reviews.llvm.org/D48845

Files:
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/FixIt/fixit-unused-lambda-capture.cpp

Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ 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; };
+  }
+};
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -993,6 +993,7 @@
   CheckCXXThisCapture(C->Loc, /*Explicit=*/true, /*BuildAndDiagnose*/ true,
   /*FunctionScopeIndexToStopAtPtr*/ nullptr,
   C->Kind == LCK_StarThis);
+  LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   continue;
 }
 
@@ -1139,6 +1140,7 @@
TryCapture_ExplicitByVal;
   tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
 }
+LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   }
   finishLambdaExplicitCaptures(LSI);
 
@@ -1478,19 +1480,22 @@
   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())
 diag << "'this'";
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();
+  diag << FixItHint::CreateRemoval(CaptureRange);
+  return true;
 }
 
 ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,18 +1537,49 @@
 
 // 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];