[PATCH] D28467: [Sema] Add warning for unused lambda captures

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Sema/ScopeInfo.h:457
+    /// lambda.
+    bool Used = false;
+
----------------
arphaman wrote:
> malcolm.parsons wrote:
> > Should this be moved into one of the `PointerIntPair`s?
> I'm not sure.. If we needed other capturing information in the future I would say that this should be its own field, but I'm not aware of anything else that's needed for this class. So I guess it would be better to pack it into `VarAndNestedAndThis`, but I wouldn't say it's a deal breaker.
I'm not keen on inconsistently initializating data members; can you perform this initialization in the constructor instead?


================
Comment at: lib/Parse/ParseExprCXX.cpp:738
 /// \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,
----------------
This change is unrelated to the patch; you can go ahead and commit it without review rather than have it as part of this patch.


================
Comment at: lib/Sema/SemaLambda.cpp:1479
     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");
----------------
Why does `From` need to be a non-const reference?


================
Comment at: lib/Sema/SemaLambda.cpp:1483
 
+      // Warn about unused explicit captures
+      if (!CurContext->isDependentContext() && !IsImplicit && !From.isUsed()) {
----------------
Missing a full stop at the end of the comment.


================
Comment at: test/SemaCXX/warn-unused-lambda-capture.cpp:17
+  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}}
+
----------------
This does not match the behavior for other -Wunused flags. e.g.,
```
void f() {
  int i;
  (void)sizeof(i);
}
```
I don't think diagnosing this test case is correct.


================
Comment at: test/SemaCXX/warn-unused-lambda-capture.cpp:59
+  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}}
+
----------------
Likewise here.


https://reviews.llvm.org/D28467