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;
+        }
       }
+      CurHasPreviousInitializer |= IsCaptureUsed;
 
       // Handle 'this' capture.
       if (From.isThisCapture()) {
@@ -1574,6 +1608,8 @@
         Init = InitResult.get();
       }
       CaptureInits.push_back(Init);
+
+      PrevCaptureLoc = EndLoc;
     }
 
     // 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,
+                                   const sema::Capture &From);
 
   /// Complete a lambda-expression having processed and attached the
   /// lambda body.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to