Don't capture variables explicitly in lambda expression.

Don't capture variables explicitly in lambda expression.

As it is, the test fails to compile on some downstream compilers with the following error:

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:25: error: lambda capture 'kPayloadLength' is not required to be captured for this use [-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:41: error: lambda capture 'kFirstSequenceNumber' is not required to be captured for this use [-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:63: error: lambda capture 'kFirstTimestamp' is not required to be captured for this use [-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                                              ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:25: error: lambda capture 'kFirstReceiveTime' is not required to be captured for this use [-Werror,-Wunused-lambda-capture]
                        kFirstReceiveTime](const SdpAudioFormat& format,

BUG=webrtc:7107
Review-Url: https://codereview.webrtc.org/2672823002
Cr-Commit-Position: refs/heads/master@{#16422}
Committed: https://chromium.googlesource.com/external/webrtc/+/b55bd5fef07f5f07438b3cafa340adb9072a619a

[ehmaldonado_webrtc, 2017-02-02 16:49:57]

Description was changed from

==========
stuff

BUG=
==========

to

==========
stuff

BUG=
==========

[ehmaldonado_webrtc, 2017-02-02 17:33:13]

Description was changed from

==========
stuff

BUG=
==========

to

==========
Don't capture variables explicitly in lambda expression.

As it is, the test fails to compile on some downstream compilers with the
following error:

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:25: error: lambda
capture 'kPayloadLength' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:41: error: lambda
capture 'kFirstSequenceNumber' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:63: error: lambda
capture 'kFirstTimestamp' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                                              ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:25: error: lambda
capture 'kFirstReceiveTime' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
                        kFirstReceiveTime](const SdpAudioFormat& format,
==========

[ehmaldonado_webrtc, 2017-02-02 17:33:23]

Patchset #1 (id:1) has been deleted

[ehmaldonado_webrtc, 2017-02-02 17:38:22]

Description was changed from

==========
Don't capture variables explicitly in lambda expression.

As it is, the test fails to compile on some downstream compilers with the
following error:

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:25: error: lambda
capture 'kPayloadLength' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:41: error: lambda
capture 'kFirstSequenceNumber' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:63: error: lambda
capture 'kFirstTimestamp' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                                              ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:25: error: lambda
capture 'kFirstReceiveTime' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
                        kFirstReceiveTime](const SdpAudioFormat& format,
==========

to

==========
Don't capture variables explicitly in lambda expression.

As it is, the test fails to compile on some downstream compilers with the
following error:

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:25: error: lambda
capture 'kPayloadLength' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:41: error: lambda
capture 'kFirstSequenceNumber' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:63: error: lambda
capture 'kFirstTimestamp' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                                              ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:25: error: lambda
capture 'kFirstReceiveTime' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
                        kFirstReceiveTime](const SdpAudioFormat& format,

BUG=webrtc:7107
==========

[ehmaldonado_webrtc, 2017-02-02 17:45:01]

ehmaldonado@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, kwiberg@webrtc.org, ossu@webrtc.org

[kwiberg-webrtc, 2017-02-02 18:24:59]

https://codereview.webrtc.org/2672823002/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc (right):

https://codereview.webrtc.org/2672823002/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:
std::unique_ptr<AudioDecoder>* dec) {
Uh? I clearly see those constants being used a bit further down in the lambda
body... whatever compiler you're using must be confused.

However. If you must make a default capture, it's probably better to capture by
reference. Otherwise, it's very hard to see exactly what sort of copy
constructors end up being called.

(Yes, that means the lambda must not be called after the end of the block in
which it was created, but that is the case here. If it weren't the case, I would
have objected to default capturing, by reference OR by value.)

[ehmaldonado_webrtc, 2017-02-02 18:47:03]

PTAL

> (Yes, that means the lambda must not be called after the end of the block in
> which it was created, but that is the case here. If it weren't the case, I
would
> have objected to default capturing, by reference OR by value.)

Can you elaborate, please?

[kwiberg-webrtc, 2017-02-02 19:01:38]

On 2017/02/02 18:47:03, ehmaldonado_webrtc wrote:
> PTAL

lgtm

> > (Yes, that means the lambda must not be called after the end of the block in
> > which it was created, but that is the case here. If it weren't the case, I
> would
> > have objected to default capturing, by reference OR by value.)
> 
> Can you elaborate, please?

A lambda that captures any local variable by reference will be the proud owner
of a dangling reference once the block in which the lambda was created ends, and
its local variables go out of scope. Using those references after that point is
bad.

The same can happen with capture-by-value---not for ordinary values, but for
pointers.

This is not so much of a problem when you list each captured variable
explicitly, since then everyone who reads the code can clearly see what stuff
the lambda references. But with default captures, you have to inspect the lambda
body carefully, and it's too easy to miss stuff. Hence the style rule about
default capture only being allowed if the lambda doesn't outlive the scope it
was created in.

[ehmaldonado_webrtc, 2017-02-02 19:03:16]

> A lambda that captures any local variable by reference will be the proud owner
> of a dangling reference once the block in which the lambda was created ends,
and
> its local variables go out of scope. Using those references after that point
is
> bad.
> 
> The same can happen with capture-by-value---not for ordinary values, but for
> pointers.
> 
> This is not so much of a problem when you list each captured variable
> explicitly, since then everyone who reads the code can clearly see what stuff
> the lambda references. But with default captures, you have to inspect the
lambda
> body carefully, and it's too easy to miss stuff. Hence the style rule about
> default capture only being allowed if the lambda doesn't outlive the scope it
> was created in.

I see. Thanks!

[ehmaldonado_webrtc, 2017-02-02 19:03:20]

The CQ bit was checked by ehmaldonado@webrtc.org

[commit-bot: I haz the power, 2017-02-02 19:03:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-02 19:51:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486062200283390,
"parent_rev": "5107246d4bf4048ca3eb428107bfccf7d757d667", "commit_rev":
"b55bd5fef07f5f07438b3cafa340adb9072a619a"}

[commit-bot: I haz the power, 2017-02-02 19:51:27]

Description was changed from

==========
Don't capture variables explicitly in lambda expression.

As it is, the test fails to compile on some downstream compilers with the
following error:

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:25: error: lambda
capture 'kPayloadLength' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:41: error: lambda
capture 'kFirstSequenceNumber' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:63: error: lambda
capture 'kFirstTimestamp' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                                              ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:25: error: lambda
capture 'kFirstReceiveTime' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
                        kFirstReceiveTime](const SdpAudioFormat& format,

BUG=webrtc:7107
==========

to

==========
Don't capture variables explicitly in lambda expression.

As it is, the test fails to compile on some downstream compilers with the
following error:

webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:25: error: lambda
capture 'kPayloadLength' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:41: error: lambda
capture 'kFirstSequenceNumber' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                        ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:316:63: error: lambda
capture 'kFirstTimestamp' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
      .WillOnce(Invoke([kPayloadLength, kFirstSequenceNumber, kFirstTimestamp,
                                                              ^
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:25: error: lambda
capture 'kFirstReceiveTime' is not required to be captured for this use
[-Werror,-Wunused-lambda-capture]
                        kFirstReceiveTime](const SdpAudioFormat& format,

BUG=webrtc:7107

Review-Url: https://codereview.webrtc.org/2672823002
Cr-Commit-Position: refs/heads/master@{#16422}
Committed:
https://chromium.googlesource.com/external/webrtc/+/b55bd5fef07f5f07438b3cafa...
==========

[commit-bot: I haz the power, 2017-02-02 19:51:29]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/b55bd5fef07f5f07438b3cafa...

[hlundin-webrtc, 2017-02-06 09:16:01]

lgtm

[ossu, 2017-02-06 14:20:51]

https://codereview.webrtc.org/2672823002/diff/20001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc (right):

https://codereview.webrtc.org/2672823002/diff/20001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/neteq_impl_unittest.cc:317:
std::unique_ptr<AudioDecoder>* dec) {
On 2017/02/02 18:24:59, kwiberg-webrtc wrote:
> Uh? I clearly see those constants being used a bit further down in the lambda
> body... whatever compiler you're using must be confused.
> 
> However. If you must make a default capture, it's probably better to capture
by
> reference. Otherwise, it's very hard to see exactly what sort of copy
> constructors end up being called.
> 
> (Yes, that means the lambda must not be called after the end of the block in
> which it was created, but that is the case here. If it weren't the case, I
would
> have objected to default capturing, by reference OR by value.)

If I were to guess as to why this happens, it's the compiler replacing constants
with literal values in the lambda body and only afterwards checking for
unnecessary captures. Very strange. Sounds like a compiler bug. Which compiler
does this? GCC? MSVC?