r291905 - [Sema] Add warning for unused lambda captures

[Nico Weber via cfe-commits]

On Mon, Jan 23, 2017 at 5:29 PM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Mon, Jan 23, 2017 at 5:00 PM, Nico Weber via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> > On Sun, Jan 22, 2017 at 6:17 AM, Malcolm Parsons <
> malcolm.parsons at gmail.com>
> > wrote:
> >>
> >> On 20 January 2017 at 21:32, Nico Weber <thakis at chromium.org> wrote:
> >> > This warns about code like
> >> >
> >> >   constexpr int foo = 4;
> >> >   [&foo]() { use(foo); }
> >> >
> >> > That's correct, but removing &foo then makes MSVC complain about this
> >> > code
> >> > like "error C3493: 'foo' cannot be implicitly captured because no
> >> > default
> >> > capture mode has been specified". A workaround is to make foo static
> >> > const
> >> > instead of constexpr.
> >> >
> >> > This seems like an MSVC bug, but it's still a bit annoying that clang
> >> > now
> >> > suggests doing things that won't build in other compilers. Any ideas
> >> > what to
> >> > do about this?
> >>
> >> Should Clang care about the behaviour of other compilers that don't
> >> follow the standard?
> >
> >
> > clang should be a compiler that people like to use. Multi-platform
> > development isn't super uncommon, so trying to give a good experience in
> > that case seems like a good thing to me -- and we've tried to do that in
> the
> > past pretty hard. It's possible that "do nothing" is the right thing, but
> > "well that other compiler is buggy" seems like a somewhat oversimplified
> > reply.
>
> We generally don't try to be bug-for-bug compatible with other
> compilers unless the bug is sufficiently important (like it is relied
> upon in system headers, for instance). In this case, I think the bug
> in MSVC is annoying, but not the end of the world because it's trivial
> to workaround in a compiler-agnostic way


It's easy, but you need to know about it. Say you're working on some
project that uses clang on macOS, gcc on Linux, MSVC on Windows, and you're
developing on Linux. You write some patch, and things work fine on your
machine. You submit your thing, it gets reverted because it triggers this
new warning on mac. Hey, fair enough, looks like a good warning, so you
remove the thing from the capture list and reland. Now you get reverted
_again_ because MSVC does need that thing that clang just told you to
revert. After three such interactions you'll be pretty unhappy, I gather :-)


> without disabling the
> diagnostic everywhere in Clang. You can leave the capture in place to
> appease MSVC and cast the use to void to appease Clang, if you so
> desire. Not an awesome solution, but my intuition is that this is an
> uncommon situation in the first place.
>
> >
> >>
> >> You could:
> >> Disable the warning on the command line.
> >> Disable the warning with a pragma.
> >
> >
> > It's an error, not a warning.
>
> I took the original statement to mean "for a Clang build", not "for an
> MSVC build".
>

Ah, that makes more sense.


>
> >
> >>
> >> Cast foo to void inside the lambda.
> >
> >
> > That's equivalent to disabling the clang warning in those cases (which
> would
> > be a shame, since the warning is super useful, and it doesn't introduce
> > problems in most cases -- only with local constexprs). Maybe the warning
> > could be split in two, so that it can be toggled separately for
> constexpr?
>
> While functional, that seems like a lot of work for this case.
>

It's like 4 lines, no?

As I said, I'm not sure it's worth it either. As-is, I think larger cross
platform teams must enable this warning for the reason mentioned above.
Requiring this is a bit of a shame since I think it's a great warning, so I
was hoping we could come up with a way so we could use the warning as well.


> However, the downside to the cast to void is that when MSVC does fix
> the bug, the diagnostic remains silenced in the user's source base.
>
> >
> >>
> >> Only capture foo when building with MSVC.
> >>
> >> Stop building with MSVC.
> >
> >
> > This reply like this make me believe you think it was silly to even
> report
> > this problem. I don't think it's silly, and there's ample precedent in
> clang
> > to make it work well in the actual world instead of in a world we wished
> we
> > lived in.
>
> Definitely not silly to report the problem, but at the same time, that
> precedent is usually reserved for things that affect a significant
> portion of users when the issue boils down to a compiler bug in
> another compiler. How common of a problem is this use case? Does it
> crop up in important header files?
>

It's not super common. Ivan, I think you ran into this twice in all of
Chromium?


>
> ~Aaron
>
> >
> >>
> >> Complain to Microsoft.
> >
> >
> > Already done. The reality is that their shipping compiler produces this
> > diagnostic, and the current 2017 build does too.
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170123/472de51f/attachment.html>