malcolm.parsons created this revision.
malcolm.parsons added reviewers: aaron.ballman, rsmith.
malcolm.parsons added a subscriber: cfe-commits.

Warn when a lambda explicitly captures something that is not used in its body.

The warning is part of -Wunused and can be enabled with -Wunused-lambda-capture.


https://reviews.llvm.org/D28467

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/ScopeInfo.h
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLambda.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p18.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p19.cpp
  test/SemaCXX/uninitialized.cpp
  test/SemaCXX/warn-unused-lambda-capture.cpp

Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-lambda-capture -Wused-but-marked-unused -Wno-uninitialized -verify -std=c++14 %s
+
+void test() {
+  int i = 0;
+
+  auto captures_nothing = [] {};
+
+  auto captures_nothing_by_value = [=] {};
+  auto captures_nothing_by_reference = [&] {};
+
+  auto implicit_by_value = [=]() mutable { i++; };
+  auto implicit_by_reference = [&] { i++; };
+
+  auto explicit_by_value_used = [i] { return i + 1; };
+  auto explicit_by_value_used_void = [i] { (void)i; };
+  auto explicit_by_value_unused = [i] {}; // expected-warning{{lambda capture 'i' is not used}}
+  auto explicit_by_value_unused_sizeof = [i] { return sizeof(i); }; // expected-warning{{lambda capture 'i' is not used}}
+
+  auto explicit_by_reference_used = [&i] { i++; };
+  auto explicit_by_reference_unused = [&i] {}; // expected-warning{{lambda capture 'i' is not used}}
+
+  auto explicit_initialized_reference_used = [&j = i] { return j + 1; };
+  auto explicit_initialized_reference_unused = [&j = i]{}; // expected-warning{{lambda capture 'j' is not used}}
+
+  auto explicit_initialized_value_used = [j = 1] { return j + 1; };
+  auto explicit_initialized_value_unused = [j = 1] {}; // expected-warning{{lambda capture 'j' is not used}}
+
+  auto nested = [&i] {
+    auto explicit_by_value_used = [i] { return i + 1; };
+    auto explicit_by_value_unused = [i] {}; // expected-warning{{lambda capture 'i' is not used}}
+  };
+}
+
+class Foo
+{
+  void test() {
+    auto explicit_this_used = [this] { return i; };
+    auto explicit_this_used_void = [this] { (void)this; };
+    auto explicit_this_unused = [this] {}; // expected-warning{{lambda capture 'this' is not used}}
+  }
+  int i;
+};
Index: test/SemaCXX/uninitialized.cpp
===================================================================
--- test/SemaCXX/uninitialized.cpp
+++ test/SemaCXX/uninitialized.cpp
@@ -1434,6 +1434,6 @@
   if (b) {
     int unused; // expected-warning {{unused variable}}
   } else {
-    [fname]{};
+    [fname] { (void)fname; }; // expected-warning {{lambda capture 'fname' is not used}}
   }
 }
Index: test/CXX/expr/expr.prim/expr.prim.lambda/p19.cpp
===================================================================
--- test/CXX/expr/expr.prim/expr.prim.lambda/p19.cpp
+++ test/CXX/expr/expr.prim/expr.prim.lambda/p19.cpp
@@ -7,7 +7,7 @@
 
 template<typename T> T &&move(T&);
 void test_special_member_functions(MoveOnly mo, int i) {
-  auto lambda1 = [i]() { }; // expected-note 2{{lambda expression begins here}} expected-note 2{{candidate}}
+  auto lambda1 = [i]() { (void)i; }; // expected-note 2{{lambda expression begins here}} expected-note 2{{candidate}}
 
   // Default constructor
   decltype(lambda1) lambda2; // expected-error{{no matching constructor}}
Index: test/CXX/expr/expr.prim/expr.prim.lambda/p18.cpp
===================================================================
--- test/CXX/expr/expr.prim/expr.prim.lambda/p18.cpp
+++ test/CXX/expr/expr.prim/expr.prim.lambda/p18.cpp
@@ -30,6 +30,8 @@
                   "should be const int&");
     static_assert(is_same<decltype((irc)), const int&>::value, 
                   "should be const int&");
+    (void)irc;
+    (void)ir;
   }();
 
   [=] {
@@ -42,5 +44,6 @@
 
   [&i] {
     static_assert(is_same<decltype((i)), int&>::value, "should be int&");
+    (void)i;
   }();
 }
Index: test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
===================================================================
--- test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
+++ test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
@@ -7,16 +7,18 @@
 };
 
 void test_capture(X x) {
-  [x] { }(); // okay: non-const copy ctor
+  [x] { (void)x; }(); // okay: non-const copy ctor
 
   [x] {
     [x] { // expected-error{{call to deleted constructor of 'X'}}
+      (void)x;
     }();
   }();
 
   [x] {
     [&x] {
       [x] { // expected-error{{call to deleted constructor of 'const X'}}
+        (void)x;
       }();
     }();
   }();
Index: test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
===================================================================
--- test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
+++ test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
@@ -3,7 +3,7 @@
 void f2() {
   int i = 1;
   void g1(int = ([i]{ return i; })()); // expected-error{{lambda expression in default argument cannot capture any entity}}
-  void g2(int = ([i]{ return 0; })()); // expected-error{{lambda expression in default argument cannot capture any entity}}
+  void g2(int = ([i]{ (void)i; return 0; })()); // expected-error{{lambda expression in default argument cannot capture any entity}}
   void g3(int = ([=]{ return i; })()); // expected-error{{lambda expression in default argument cannot capture any entity}}
   void g4(int = ([=]{ return 0; })());
   void g5(int = ([]{ return sizeof i; })());
Index: test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
===================================================================
--- test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
+++ test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
@@ -2,7 +2,7 @@
 
 void odr_used() {
   int i = 17;
-  [i]{}();
+  [i]{}(); // expected-warning{{lambda capture 'i' is not used}}
 }
 
 struct ReachingThis {
@@ -13,14 +13,14 @@
       int i;
 
       void bar() {
-        (void)[this](){};
+        (void)[this](){}; // expected-warning{{lambda capture 'this' is not used}}
         (void)[&](){i = 7; };
       }
     };
   }
 
   void foo() {
-    (void)[this](){};
+    (void)[this](){}; // expected-warning{{lambda capture 'this' is not used}}
     
     struct Local {
       int i;
@@ -35,11 +35,11 @@
 
 void immediately_enclosing(int i) { // expected-note{{'i' declared here}}
   [i]() {
-    [i] {}();
+    [i] {}(); // expected-warning{{lambda capture 'i' is not used}}
   }();
 
   [=]() {
-    [i] {}();
+    [i] {}(); // expected-warning{{lambda capture 'i' is not used}}
   }();
 
   []() { // expected-note{{lambda expression begins here}}
@@ -71,6 +71,7 @@
           x += i; // expected-error{{variable 'i' cannot be implicitly captured in a lambda with no capture-default specified}}
           x += f;
         };
+        (void)this;
       };
     } 
   };
Index: lib/Sema/SemaLambda.cpp
===================================================================
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1476,10 +1476,19 @@
     // Translate captures.
     auto CurField = Class->field_begin();
     for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) {
-      LambdaScopeInfo::Capture From = LSI->Captures[I];
+      LambdaScopeInfo::Capture &From = LSI->Captures[I];
       assert(!From.isBlockCapture() && "Cannot capture __block variables");
       bool IsImplicit = I >= LSI->NumExplicitCaptures;
 
+      // Warn about unused explicit captures
+      if (!IsImplicit && !From.isUsed()) {
+        auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
+        if (From.isThisCapture())
+          diag << "'this'";
+        else
+          diag << From.getVariable();
+      }
+
       // Handle 'this' capture.
       if (From.isThisCapture()) {
         Captures.push_back(
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1106,6 +1106,8 @@
             dyn_cast<CapturingScopeInfo>(FunctionScopes[idx])) {
       if (CSI->CXXThisCaptureIndex != 0) {
         // 'this' is already being captured; there isn't anything more to do.
+        if (BuildAndDiagnose)
+          CSI->Captures[CSI->CXXThisCaptureIndex - 1].markUsed();
         break;
       }
       LambdaScopeInfo *LSI = dyn_cast<LambdaScopeInfo>(CSI);
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13881,8 +13881,11 @@
 
     // Check whether we've already captured it.
     if (isVariableAlreadyCapturedInScopeInfo(CSI, Var, Nested, CaptureType, 
-                                             DeclRefType)) 
+                                             DeclRefType)) {
+      if (BuildAndDiagnose)
+        CSI->getCapture(Var).markUsed();
       break;
+    }
     // If we are instantiating a generic lambda call operator body, 
     // we do not want to capture new variables.  What was captured
     // during either a lambdas transformation or initial parsing
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -735,7 +735,7 @@
 ///        sometimes skip the initializers for init-captures and not fully
 ///        populate \p Intro. This flag will be set to \c true if we do so.
 /// \return A DiagnosticID if it hit something unexpected. The location for
-///         for the diagnostic is that of the current token.
+///         the diagnostic is that of the current token.
 Optional<unsigned> Parser::ParseLambdaIntroducer(LambdaIntroducer &Intro,
                                                  bool *SkippedInits) {
   typedef Optional<unsigned> DiagResult;
Index: include/clang/Sema/ScopeInfo.h
===================================================================
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -452,6 +452,10 @@
     /// non-static data member that would hold the capture.
     QualType CaptureType;
 
+    /// \brief Whether an explicit capture has been used in the body of the
+    /// lambda.
+    bool Used = false;
+
   public:
     Capture(VarDecl *Var, bool Block, bool ByRef, bool IsNested,
             SourceLocation Loc, SourceLocation EllipsisLoc,
@@ -491,6 +495,8 @@
     bool isNested() const {
       return VarAndNestedAndThis.getInt() & IsNestedCapture;
     }
+    bool isUsed() const { return Used; }
+    void markUsed() { Used = true; }
 
     VarDecl *getVariable() const {
       return VarAndNestedAndThis.getPointer();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -316,6 +316,8 @@
   InGroup<UnneededMemberFunction>, DefaultIgnore;
 def warn_unused_private_field: Warning<"private field %0 is not used">,
   InGroup<UnusedPrivateField>, DefaultIgnore;
+def warn_unused_lambda_capture: Warning<"lambda capture %0 is not used">,
+  InGroup<UnusedLambdaCapture>, DefaultIgnore;
 
 def warn_parameter_size: Warning<
   "%0 is a large (%1 bytes) pass-by-value argument; "
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -480,6 +480,7 @@
 def UnusedMemberFunction : DiagGroup<"unused-member-function",
                                      [UnneededMemberFunction]>;
 def UnusedLabel : DiagGroup<"unused-label">;
+def UnusedLambdaCapture : DiagGroup<"unused-lambda-capture">;
 def UnusedParameter : DiagGroup<"unused-parameter">;
 def UnusedResult : DiagGroup<"unused-result">;
 def PotentiallyEvaluatedExpression : DiagGroup<"potentially-evaluated-expression">;
@@ -616,8 +617,9 @@
                        [UnusedArgument, UnusedFunction, UnusedLabel,
                         // UnusedParameter, (matches GCC's behavior)
                         // UnusedMemberFunction, (clean-up llvm before enabling)
-                        UnusedPrivateField, UnusedLocalTypedef,
-                        UnusedValue, UnusedVariable, UnusedPropertyIvar]>,
+                        UnusedPrivateField, UnusedLambdaCapture,
+                        UnusedLocalTypedef, UnusedValue, UnusedVariable,
+                        UnusedPropertyIvar]>,
                         DiagCategory<"Unused Entity Issue">;
 
 // Format settings.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to