r291905 - [Sema] Add warning for unused lambda captures

[Aaron Ballman via cfe-commits]

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 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".

>
>>
>> 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.
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?

~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
>