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

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.


================
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}}
+
----------------
Quuxplusone wrote:
> malcolm.parsons wrote:
> > Quuxplusone wrote:
> > > aaron.ballman wrote:
> > > > 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.
> > > I see some value to maybe diagnosing *something* here. For example, `[] { return sizeof(i); }` would produce the same result as `[i] { return sizeof(i); }` but with one fewer capture, so removing the `i` might be seen as an improvement.
> > > 
> > > But I'm not sure how to convey this information to the user. You could say "variable `i` used in unevaluated context refers to the `i` in the outer scope, not the captured `i`"... except that I think that would be false. Given that you *have* captured an `i`, `sizeof(i)` definitely refers to the captured one AFAIK. The fact that the captured `i` shadows an `i` in the outer scope is irrelevant --- in fact the user is *expecting* to shadow the outer `i`.
> > > 
> > > Perhaps it would be appropriate to reword the diagnostic to "lambda captures variable `i` unnecessarily".  I would also lean toward splitting it into two diagnostics — one for "this capture is unnecessary" (as in this example) and one for "this capture doesn't even appear lexically in the body of the lambda". Not sure how other people would feel about that.
> > > 
> > > You should add some test cases with `decltype(i)` for the same reason as `sizeof(i)`.
> > Does "lambda capture 'i' is not odr-used" make more sense?
> > Does "lambda capture 'i' is not odr-used" make more sense?
> 
> That would completely satisfy *me*, for what that's worth. It admittedly doesn't match the other -Wunused diagnostics, but it is concise and correct — at least I assume it's correct. :)
C++14 [expr.prim.lambda]p18:

> [ Note: An id-expression that is not an odr-use refers to the original entity, never to a member
> of the closure type. Furthermore, such an id-expression does not cause the implicit capture of the entity.
> — end note ]


https://reviews.llvm.org/D28467