C++11 Feature Proposal: Lambdas

[Alex Vakulenko]

What:

Anonymous function objects (lambdas) allow declaring simple inline functions. This is especially useful with generic algorithms.

http://en.wikipedia.org/wiki/C%2B%2B11#Lambda_functions_and_expressions

Why:

Enable easy usage of generic algorithms, simple callbacks, and callback adapters without having to create stand-alone functor objects or functions.

Example:

1. Convert an array of strings to lowercase:

std::vector<string> lines = GetTextLines();

std::for_each(lines.begin(), lines.end(), 

              [](string& s) { base::StringToLowerASCII(&s); });

2. Given an array of integers, move all odd numbers to the front of the array:

std::vector<int> numbers{1, 2, 3, 4, 5, 6, 7, 8};

std::stable_partition(numbers.begin(), numbers.end(),

                      [](int n) { return (n % 2) != 0; });

Lambdas/functors can be easily plugged into the existing Chrome's Callback/Bind system. I have already created a bind adapter for this in Chrome OS: https://chromium.googlesource.com/chromiumos/platform2/+/master/libchromeos/chromeos/bind_lambda.h

base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind([] {

  LOG(INFO) << "Task executed asynchronously";

}));

Where:

In places where very simple function objects are required.

[Nico Weber]

Is it possible to write a function that takes a lambda if we don't have std::function? (This requires library support, which we don't have.)

Your examples use std::for_each with a lambda and std::stable_partition with a lambda – since we don't have c++11 standard library support, neither of these examples will work.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[James Robinson]

On Wed, Sep 24, 2014 at 3:55 PM, Alex Vakulenko <avaku...@chromium.org> wrote:

What:

Anonymous function objects (lambdas) allow declaring simple inline functions. This is especially useful with generic algorithms.

http://en.wikipedia.org/wiki/C%2B%2B11#Lambda_functions_and_expressions

Why:

Enable easy usage of generic algorithms, simple callbacks, and callback adapters without having to create stand-alone functor objects or functions.

Example:

1. Convert an array of strings to lowercase:

std::vector<string> lines = GetTextLines();

std::for_each(lines.begin(), lines.end(), 

              [](string& s) { base::StringToLowerASCII(&s); });

This won't work without library support, which we don't have.

 

2. Given an array of integers, move all odd numbers to the front of the array:

std::vector<int> numbers{1, 2, 3, 4, 5, 6, 7, 8};

std::stable_partition(numbers.begin(), numbers.end(),

                      [](int n) { return (n % 2) != 0; });

This won't work without library support, which we don't have.

Could everyone please stick to examples will which actually work in chromium in their proposals?  It doesn't help anyone to consider examples that do not actually compile.

Lambdas/functors can be easily plugged into the existing Chrome's Callback/Bind system. I have already created a bind adapter for this in Chrome OS: https://chromium.googlesource.com/chromiumos/platform2/+/master/libchromeos/chromeos/bind_lambda.h

This uses several C++11 features that aren't on the allowed list and would have to be proposed first, such as decltype() and variadric templates.

In general lambda isn't useful without at least some of auto, std::function, decltype(), and standard library algorithm support.  We don't have any of those yet so proposing lambdas seems to be jumping the gun.

- James

[Alex Vakulenko]

Hold on, I'm a bit confused here. So you are saying we don't have any STL support (even the one from C++03)? Where do std::string and std::vector come from then?

std::for_each and std::stable_partition were part of standard libraries long before C+11. And they take functors, which is what lambda are as well. So, I'm not sure how is using lambdas any different with these algorithms vs simple functions or functor objecs?

[Jeffrey Yasskin]

Without std::function or something equivalent like Alex's https://chromium.googlesource.com/chromiumos/platform2/+/master/libchromeos/chromeos/bind_lambda.h, any function taking a lambda has to be a template. But any template taking a C++98 functor should be fine taking a lambda instead.

I haven't investigated if lambdas themselves are implemented using any code that needs to be shipped with the standard library, but I'd be a little surprised if they were.

[James Robinson]

My apologies, I didn't realize a C++03 implementation of std::for_each would be able to handle the lambda->functor conversion, but it appears to work (at least on android): https://codereview.chromium.org/598273002

The point about base::Bind still applies, though.  How does your adapter handle capture and things like base::Unretained() ?

[Mostyn Bramley-Moore]

A lambda snuck into mainline recently, until I replaced it with c++03 code:
https://codereview.chromium.org/565443002/

So that much seemed to work, at least.

-Mostyn.

On 09/25/2014 01:35 AM, 'James Robinson' via Chromium-dev wrote:
> My apologies, I didn't realize a C++03 implementation of std::for_each
> would be able to handle the lambda->functor conversion, but it appears
> to work (at least on android): https://codereview.chromium.org/598273002
>
> The point about base::Bind still applies, though. How does your adapter
> handle capture and things like base::Unretained() ?
>
> On Wed, Sep 24, 2014 at 4:16 PM, Alex Vakulenko <avaku...@chromium.org
> <mailto:avaku...@chromium.org>> wrote:
>
> Hold on, I'm a bit confused here. So you are saying we don't have
> any STL support (even the one from C++03)? Where do std::string and
> std::vector come from then?
>
> std::for_each and std::stable_partition were part of standard
> libraries long before C+11. And they take functors, which is what
> lambda are as well. So, I'm not sure how is using lambdas any
> different with these algorithms vs simple functions or functor objecs?
>
> On Wed, Sep 24, 2014 at 4:04 PM, James Robinson <jam...@chromium.org
> <mailto:jam...@chromium.org>> wrote:
>
>
>
> On Wed, Sep 24, 2014 at 3:55 PM, Alex Vakulenko

> <avaku...@chromium.org <mailto:avaku...@chromium.org>> wrote:
>
> *What:*

>
> Anonymous function objects (lambdas) allow declaring simple
> inline functions. This is especially useful with generic
> algorithms.
>
> http://en.wikipedia.org/wiki/C%2B%2B11#Lambda_functions_and_expressions
>

> *Why:*

> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
>

> To unsubscribe from this group and stop receiving emails from it, send
> an email to chromium-dev...@chromium.org
> <mailto:chromium-dev...@chromium.org>.


--
Mostyn Bramley-Moore
TV and Connected Devices
Opera Software ASA
mos...@opera.com

[Nico Weber]

On Wed, Sep 24, 2014 at 4:33 PM, Jeffrey Yasskin <jyas...@chromium.org> wrote:

Without std::function or something equivalent like Alex's https://chromium.googlesource.com/chromiumos/platform2/+/master/libchromeos/chromeos/bind_lambda.h, any function taking a lambda has to be a template. But any template taking a C++98 functor should be fine taking a lambda instead.

I haven't investigated if lambdas themselves are implemented using any code that needs to be shipped with the standard library, but I'd be a little surprised if they were.

Oh cool, I didn't know that :-)

Since lambdas are a somewhat larger feature, I'd like to postpone this proposal for a bit (say, two weeks) so that we have some time to get used to the smaller features first. Is that ok?

[Alex Vakulenko]

Yes, the lambda captures seem to be handled with base::Bind adapter. Also, lambda without any capture decays to just function pointer, so you can do this:

int foo(int(*func)(int, int)) {

  return func(10, 20);

}

int main() {

  printf("a = %d\n", foo([](int x, int y) { return x + y; }));

  printf("b = %d\n", foo([](int x, int y) { return x * y; }));

  return 0;

}

This prints:

a = 30

b = 200

The reason why this works is quite simple. On all compilers that I know of, lambdas are implemented as a simple struct. Here is an example:

As in the previous example, the following code:

int main() {

  auto my_lambda = [](int x, int y) {

    return x + y;

  };

  int a = my_lambda(3, 5);

  return 0;

}

Is essentially transformed into the following by the compiler:

struct lambda_8b5a7c65366141b27089e0ffb05f3138 {

  int operator()(int x, int y) {

    return x + y;

  }

};

int main() {

  lambda_8b5a7c65366141b27089e0ffb05f3138 my_lambda;

  int a = my_lambda(3, 5);

  return 0;

}

In fact, when lambda isn't capturing any data, the operator() is actually static (which you can't do in normal struct), so you can actually get a function pointer to it.

When you capture something in lambda, the captures go to struct's members:

int main() {

  int some_data = 7;

  auto my_lambda = [some_data](int x, int y) {

    return x + y + some_data;

  };

  int a = my_lambda(3, 5);

  return 0;

}

Essentially would become:

struct lambda_3245f7c65323453451b27089e0ffb05f3138 {

  int operator()(int x, int y) {

    return x + y + some_data;

  }

  lambda_3245f7c65323453451b27089e0ffb05f3138(int v1) : some_data(v1) {}

  int some_data;

};

int main() {

  int some_data = 7;

  lambda_3245f7c65323453451b27089e0ffb05f3138 my_lambda(some_data);

  int a = my_lambda(3, 5);

  return 0;

}

So, the differences from before are:

1. Lamda object now has data associated with it
2. It has a constructor that takes the data from outside and stored it in lambda object
3. operator() is no longer static (since it needs access to member variables, so it needs this pointer), so lambdas with captures cannot be used in lieu of simple function pointers.

As you can see, this is language-only feature and doesn't require any library support. In fact, a lambda is nothing more than a compiler-generated struct with operator(), that is a normal C++ functor object.

I hope this helps.

[James Robinson]

On Wed, Sep 24, 2014 at 4:59 PM, Alex Vakulenko <avaku...@chromium.org> wrote:

Yes, the lambda captures seem to be handled with base::Bind adapter. Also, lambda without any capture decays to just function pointer, so you can do this:

int foo(int(*func)(int, int)) {

  return func(10, 20);

}

int main() {

  printf("a = %d\n", foo([](int x, int y) { return x + y; }));

  printf("b = %d\n", foo([](int x, int y) { return x * y; }));

  return 0;

}

This prints:

a = 30

b = 200

OK, this makes sense.  I've played with it a bit and the base::Unretained/ConstRef appear to work the same way, for the reasoning you apply below which makes sense.

One thing about lambdas that is not obvious is that while they can be converted to functor-like types (like your base::Bind adapter does) the actual type of the lambda can't be spelled out, so you can't efficiently store a lambda in a variable without using 'auto'.  How about we let the auto thread resolve then enable lambdas?

- James

[Alex Vakulenko]

How about we let the auto thread resolve then enable lambdas?

I've waited for so long, I can wait a little bit more :)

[Klemen Forstnerič]

Hi everyone,

seeing that auto got enabled, what's the status of lambdas? :-) 

Cheers,

Klemen

[David Michael]

On Thu, Sep 25, 2014 at 12:40 AM, James Robinson <jam...@chromium.org> wrote:

On Wed, Sep 24, 2014 at 4:59 PM, Alex Vakulenko <avaku...@chromium.org> wrote:

Yes, the lambda captures seem to be handled with base::Bind adapter. Also, lambda without any capture decays to just function pointer, so you can do this:

int foo(int(*func)(int, int)) {

  return func(10, 20);

}

int main() {

  printf("a = %d\n", foo([](int x, int y) { return x + y; }));

  printf("b = %d\n", foo([](int x, int y) { return x * y; }));

  return 0;

}

This prints:

a = 30

b = 200

OK, this makes sense.  I've played with it a bit and the base::Unretained/ConstRef appear to work the same way, for the reasoning you apply below which makes sense.

It makes sense that things like base::Unretained and base::Owned work if you remember them. But that's kind of beside the point of those. By replacing usages of Bind with lambdas and captures, we'd be giving up the protections of base::Bind. Currently base::Bind will actually fail if you try to bind a raw pointer without specifying ownership. I haven't played with lambdas, but I assume they happily capture a raw pointer.

base::Bind also does detection of RefCounted types and ref-counts the target object for member functions by default.

I think if we allow lambdas, it would probably be wise (at least initially) to only allow them to be defined and used within the scope of a function, and not passed in or out of a function. That way, they can be used in things like STL algorithms, but we don't give up the safety net that base::Bind gives us.

 

One thing about lambdas that is not obvious is that while they can be converted to functor-like types (like your base::Bind adapter does) the actual type of the lambda can't be spelled out, so you can't efficiently store a lambda in a variable without using 'auto'.  How about we let the auto thread resolve then enable lambdas?

- James

--

[Ryan Sleevi]

On Oct 21, 2014 8:09 AM, "David Michael" <dmic...@chromium.org> wrote:
>
> On Thu, Sep 25, 2014 at 12:40 AM, James Robinson <jam...@chromium.org> wrote:
>>
>>
>>
>> On Wed, Sep 24, 2014 at 4:59 PM, Alex Vakulenko <avaku...@chromium.org> wrote:
>>>
>>> Yes, the lambda captures seem to be handled with base::Bind adapter. Also, lambda without any capture decays to just function pointer, so you can do this:
>>>
>>> int foo(int(*func)(int, int)) {
>>>   return func(10, 20);
>>> }
>>>
>>> int main() {
>>>   printf("a = %d\n", foo([](int x, int y) { return x + y; }));
>>>   printf("b = %d\n", foo([](int x, int y) { return x * y; }));
>>>   return 0;
>>> }
>>>
>>> This prints:
>>> a = 30
>>> b = 200
>>>
>>
>> OK, this makes sense.  I've played with it a bit and the base::Unretained/ConstRef appear to work the same way, for the reasoning you apply below which makes sense.
>
> It makes sense that things like base::Unretained and base::Owned work if you remember them. But that's kind of beside the point of those. By replacing usages of Bind with lambdas and captures, we'd be giving up the protections of base::Bind. Currently base::Bind will actually fail if you try to bind a raw pointer without specifying ownership. I haven't played with lambdas, but I assume they happily capture a raw pointer.
>
> base::Bind also does detection of RefCounted types and ref-counts the target object for member functions by default.
>
> I think if we allow lambdas, it would probably be wise (at least initially) to only allow them to be defined and used within the scope of a function, and not passed in or out of a function. That way, they can be used in things like STL algorithms, but we don't give up the safety net that base::Bind gives us.
>  

Isn't it true that lambdas for use with STL algorithms require std::function, a library feature (ergo not universal)?

[David Michael]

On Tue, Oct 21, 2014 at 9:21 AM, Ryan Sleevi <rsl...@chromium.org> wrote:

On Oct 21, 2014 8:09 AM, "David Michael" <dmic...@chromium.org> wrote:
>
> On Thu, Sep 25, 2014 at 12:40 AM, James Robinson <jam...@chromium.org> wrote:
>>
>>
>>
>> On Wed, Sep 24, 2014 at 4:59 PM, Alex Vakulenko <avaku...@chromium.org> wrote:
>>>
>>> Yes, the lambda captures seem to be handled with base::Bind adapter. Also, lambda without any capture decays to just function pointer, so you can do this:
>>>
>>> int foo(int(*func)(int, int)) {
>>>   return func(10, 20);
>>> }
>>>
>>> int main() {
>>>   printf("a = %d\n", foo([](int x, int y) { return x + y; }));
>>>   printf("b = %d\n", foo([](int x, int y) { return x * y; }));
>>>   return 0;
>>> }
>>>
>>> This prints:
>>> a = 30
>>> b = 200
>>>
>>
>> OK, this makes sense.  I've played with it a bit and the base::Unretained/ConstRef appear to work the same way, for the reasoning you apply below which makes sense.
>
> It makes sense that things like base::Unretained and base::Owned work if you remember them. But that's kind of beside the point of those. By replacing usages of Bind with lambdas and captures, we'd be giving up the protections of base::Bind. Currently base::Bind will actually fail if you try to bind a raw pointer without specifying ownership. I haven't played with lambdas, but I assume they happily capture a raw pointer.
>
> base::Bind also does detection of RefCounted types and ref-counts the target object for member functions by default.
>
> I think if we allow lambdas, it would probably be wise (at least initially) to only allow them to be defined and used within the scope of a function, and not passed in or out of a function. That way, they can be used in things like STL algorithms, but we don't give up the safety net that base::Bind gives us.
>  

Isn't it true that lambdas for use with STL algorithms require std::function, a library feature (ergo not universal)?

The STL algorithms that require a function-like thing generally accept a functor whose type is provided and inferred via a template parameter. See section 25 of the standard:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf

They don't use std::function directly, although std::function and std::bind are convenient ways to provide a functor to algorithms. Lambdas should work fine there without use of std::function.

I'm OK with not having lambdas, mostly due to the dangers I described above (and because the syntax is disgusting). My main point was if we *do* accept lambdas, I would prefer we limit their usage to avoid some of the pitfalls.

[Jeremy Roman]

On Tue, Oct 21, 2014 at 11:21 AM, Ryan Sleevi <rsl...@chromium.org> wrote:
> Isn't it true that lambdas for use with STL algorithms require
> std::function, a library feature (ergo not universal)?

No, they do not. The lambda's type (which is not std::function, but
rather an autogenerated type that you cannot explicitly write in
source code) will be inferred by the template (and as a result, the
lambda's body may well be inlined in the template instantiation). See
also jyasskin and jamesr's posts on Sept 24.

It's useful to think of this:

void foo() {
int x = 4;
std::for_each(ys.begin(), ys.end(), [x](int y) { printf("%d\n", x+y); });
}

as (roughly) equivalent to:

void foo() {
int x = 4;
class __lambda0 {
public:
__lambda0(int x) : x_(x) {}
void operator()(int y) { printf("%d\n", x_+y); }
private:
int x_;
};
std::for_each(ys.begin(), ys.end(), __lambda0(x));
}

See also the cppreference description of lambdas (or for even more
detail, the standard).
http://en.cppreference.com/w/cpp/language/lambda

[Daniel Murphy]

Casually jumping in here to voice my "yes please" for lambdas.  They will hugely simplify a lot of my async code and make it much more readable

[Krzysztof Olczyk]

Hi all,

I would like to second that "yes please" voice for lambdas.

They, indeed, simplify the code and remove the noise of having

to declare explicit functors for simple operations.

As Jeremy has said, no C++11 standard library functionality is needed

for lambda expressions. 

They are simply the compiler-generated "non-union classes which have const public inline function call operator" which have "an implicitly-declared copy constructor"

and each capture becomes a "non-static data member of the resulting closure object".

More details are available in "5.1.2 Lambda expressions" of C++ standard.

Therefore, they can be passed everywhere where functor with templated type is expected. 

This includes C++03 stdlib algorithms like std::for_each or std::transform. 

Additionally, if lambda expression does not capture any state, it can be implicitly

converted to plain function pointer (5.1.2.6).

We could also make it possible to bind lambda expressions to base::Callback<>,

to be able to pass them around where base::Callback<> is expected and also,

possibly, compensate for the lack of std::function<>.

The demonstrative CL showing how it could be achieved:

https://codereview.chromium.org/694453002

It seems to work on all Chromium compilers.

So are there any concerns not to include lambdas in the list of allowed features?

-- 

Best regards,

Krzysztof Olczyk

Opera Software ASA

TV & Devices

On Wed, Oct 29, 2014 at 11:30 PM, Daniel Murphy <dmu...@chromium.org> wrote:

Casually jumping in here to voice my "yes please" for lambdas.  They will hugely simplify a lot of my async code and make it much more readable

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Maciej Pawlowski]

Another +1.

W dniu 2014-10-30 o 11:20, Krzysztof Olczyk pisze:

-- 
BR
Maciej Pawlowski
Opera Desktop Wroclaw

[David Michael]

On Thu, Oct 30, 2014 at 4:20 AM, Krzysztof Olczyk <kol...@opera.com> wrote:

Hi all,

I would like to second that "yes please" voice for lambdas.

They, indeed, simplify the code and remove the noise of having

to declare explicit functors for simple operations.

As Jeremy has said, no C++11 standard library functionality is needed

for lambda expressions. 

They are simply the compiler-generated "non-union classes which have const public inline function call operator" which have "an implicitly-declared copy constructor"

and each capture becomes a "non-static data member of the resulting closure object".

More details are available in "5.1.2 Lambda expressions" of C++ standard.

Therefore, they can be passed everywhere where functor with templated type is expected. 

This includes C++03 stdlib algorithms like std::for_each or std::transform. 

Additionally, if lambda expression does not capture any state, it can be implicitly

converted to plain function pointer (5.1.2.6).

We could also make it possible to bind lambda expressions to base::Callback<>,

to be able to pass them around where base::Callback<> is expected and also,

possibly, compensate for the lack of std::function<>.

The demonstrative CL showing how it could be achieved:

https://codereview.chromium.org/694453002

It seems to work on all Chromium compilers.

So are there any concerns not to include lambdas in the list of allowed features?

I'm still concerned about the fact that lambdas will allow us to bind raw pointers without any thoughts to lifetime. As I noted before, base::Callback ensures that you specify base::Owned or base::Unretained, and also ensures that base::RefCounted pointers get ref-counted.

If we allow lambdas, I would prefer they were only allowed within the scope of a function, and are never passed in or out, to protect against most lifetime mistakes. We could see if that is sufficient for most uses and go from there.

Also, it's actually kind of rare that I find myself wishing I could use one, and the syntax is abominable. That could change over time.

[Jeffrey Yasskin]

One simple, but inconvenient, way to address this would be to disallow
lambdas that capture anything. That is, make people write code like:

MyAsyncFunction(base::Bind([](RefcountedClass* self, Arg1* a1, Arg2*
a2, Arg3 a3) {
// body
}, this, base::Owned(local_a1), base::Unretained(local_a2), local_a3));

If you see anything inside the []s, you'd know it was against the style guide.

Even with the inconvenience of having to retype all the argument
types, this would still be easier to read and more convenient in many
cases than having to define a completely separate function.

In the long run, we might be able to require that all lambdas use [=]
capture (instead of [&]) and that nothing captured is a raw pointer.
However, for "Owned" captures, we need to be able to capture
std::unique_ptr<>s by move, which won't come until C++14 and MSVC14:
init-captures in
http://blogs.msdn.com/b/vcblog/archive/2014/06/11/c-11-14-feature-tables-for-visual-studio-14-ctp1.aspx.

> If we allow lambdas, I would prefer they were only allowed within the scope
> of a function, and are never passed in or out, to protect against most
> lifetime mistakes. We could see if that is sufficient for most uses and go
> from there.

I'm having trouble seeing why you'd think that there's any chance this
would be sufficient for most uses. Do you commonly create Closures
that you don't pass to any other function?

[Maciej Pawlowski]

W dniu 2014-10-30 o 16:11, David Michael pisze:

> I'm still concerned about the fact that lambdas will allow us to bind
> raw pointers without any thoughts to lifetime. As I noted before,
> base::Callback ensures that you specify base::Owned or
> base::Unretained, and also ensures that base::RefCounted pointers get
> ref-counted.
>
> If we allow lambdas, I would prefer they were only allowed within the
> scope of a function, and are never passed in or out, to protect
> against most lifetime mistakes. We could see if that is sufficient for
> most uses and go from there.

Agreed, this is a concern. I think there should be some guidance in the
style guide about it, but the feature is too useful in general to
abandon it IMO.

Using short lambdas in local scope, for STL algorithms or whatnot,
sounds like a no-brainer to me. It's clearer to read and easier to write
than having an anonymous namespace at the top of the file and a random
predicate (usually with a comment like "for the std::sort in function X").

As for larger scopes, perhaps they could be allowed without the default
captures that could bind an unguarded 'this'? So, if one wanted to have
a long-lived lambda, he'd be required to capture 'this' explicitly
(which would be equivalent to base::Unretained). Callbacks are still
more explicit/safer about it, yeah.
Maybe all captures of 'this' should be wrapped in some little class that
seamlessly allows RefCounted classes and weak_ptrs but disallows other
bare pointers, unless also passed through base::Unretained? Something
like in Callback. Haven't given this much thought.

> Also, it's actually kind of rare that I find myself wishing I could
> use one, and the syntax is abominable. That could change over time.

Whether one finds the syntax of a feature pretty or not, or whether one
finds himself personally using it or not, should be irrelevant in terms
of allowing or banning it. For example, I wished for lambdas many times
when writing find_ifs and the syntax doesn't bother me.

Generally, IMHO the default approach to allowing/banning a C++11 feature
should be "Allow unless it's really bad", not "Ban unless proven really
good". We want to be as close to standard C++ as possible within the
constraint of the core principles of the style guide, not end up with a
small subset of the language because there was once a couple of guys who
didn't think they'd be needing feature X. Any rule (banned feature) is
an organizational cost required to enforce it (and circumvent it when
needed), new rules should be introduced only when not doing so would be
even costlier.

[David Michael]

...also may be easy to enforce with the Clang plugin, which is nice. But I don't necessarily want to restrict (explicit) captures entirely for lambdas within a function scope.

Even with the inconvenience of having to retype all the argument
types, this would still be easier to read and more convenient in many
cases than having to define a completely separate function.

In the long run, we might be able to require that all lambdas use [=]
capture (instead of [&]) and that nothing captured is a raw pointer.

Currently, the Google style guide doesn't allow default captures, so I expect we'll be no more permissive than that.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Lambda_expressions

...but maybe that's part of what you mean by "long run".

 

However, for "Owned" captures, we need to be able to capture
std::unique_ptr<>s by move, which won't come until C++14 and MSVC14:
init-captures in
http://blogs.msdn.com/b/vcblog/archive/2014/06/11/c-11-14-feature-tables-for-visual-studio-14-ctp1.aspx.

> If we allow lambdas, I would prefer they were only allowed within the scope
> of a function, and are never passed in or out, to protect against most
> lifetime mistakes. We could see if that is sufficient for most uses and go
> from there.

I'm having trouble seeing why you'd think that there's any chance this
would be sufficient for most uses. Do you commonly create Closures
that you don't pass to any other function?

The times when I've wished for lambdas have been where I wanted to use something from <algorithm>, but didn't want to define a new, separate function. If I'm making a Closure/Callback in Chromium for something that will be invoked at a later time or on another thread, I think it's sensible to make a separate function or method for the sake of readability.

I could be in the minority, and I admittedly don't have experience with lambdas in C++ yet.

[David Michael]

On Thu, Oct 30, 2014 at 9:48 AM, Maciej Pawlowski <mpawl...@opera.com> wrote:

W dniu 2014-10-30 o 16:11, David Michael pisze:

I'm still concerned about the fact that lambdas will allow us to bind raw pointers without any thoughts to lifetime. As I noted before, base::Callback ensures that you specify base::Owned or base::Unretained, and also ensures that base::RefCounted pointers get ref-counted.

If we allow lambdas, I would prefer they were only allowed within the scope of a function, and are never passed in or out, to protect against most lifetime mistakes. We could see if that is sufficient for most uses and go from there.

Agreed, this is a concern. I think there should be some guidance in the style guide about it, but the feature is too useful in general to abandon it IMO.

There's a big difference between having static, compile-time safety checks and having guidance. I'm not suggesting abandoning the feature, necessarily. I'm suggesting limiting it for the sake of keeping our safety net for memory errors. In my experience, object lifetime issues are probably the most frequent cause of crashes and security bugs in Chromium.

Using short lambdas in local scope, for STL algorithms or whatnot, sounds like a no-brainer to me. It's clearer to read and easier to write than having an anonymous namespace at the top of the file and a random predicate (usually with a comment like "for the std::sort in function X").

I don't know if it's a "no-brainer"; it's another concept and new syntax that Chromium committers would have to be able to review and understand. But I think I agree it's probably worth the tradeoff in the long run, for this kind of usage.
 

As for larger scopes, perhaps they could be allowed without the default captures that could bind an unguarded 'this'? So, if one wanted to have a long-lived lambda, he'd be required to capture 'this' explicitly (which would be equivalent to base::Unretained).

Agree...  the Google style guide disallows default captures, and we should be at least as restrictive:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Lambda_expressions

 

Callbacks are still more explicit/safer about it, yeah.
Maybe all captures of 'this' should be wrapped in some little class that seamlessly allows RefCounted classes and weak_ptrs but disallows other bare pointers, unless also passed through base::Unretained? Something like in Callback. Haven't given this much thought.

The downside is it would still be possible to forget. But if it was in the style guide and written that way consistently in the codebase, that might suffice. Raw pointer params might still be easy to forget; not sure.

 

Also, it's actually kind of rare that I find myself wishing I could use one, and the syntax is abominable. That could change over time.

Whether one finds the syntax of a feature pretty or not, or whether one finds himself personally using it or not, should be irrelevant in terms of allowing or banning it. For example, I wished for lambdas many times when writing find_ifs and the syntax doesn't bother me.

I can only speak for myself, so consider it a data point and an opportunity for others to comment.

 

Generally, IMHO the default approach to allowing/banning a C++11 feature should be "Allow unless it's really bad", not "Ban unless proven really good".

I don't agree with either of those approaches. It should be "How can we get the most value out of this feature?" That might mean "don't use it at all", "use it whenever you want", or "only uses in these narrow circumstances". There are plenty of C++ features that are "mostly harmless" we don't use, because we optimize for readability and maintainability. The language is too big to allow everything in a sane project.

 

We want to be as close to standard C++ as possible within the constraint of the core principles of the style guide, not end up with a small subset of the language because there was once a couple of guys who didn't think they'd be needing feature X. Any rule (banned feature) is an organizational cost required to enforce it (and circumvent it when needed), new rules should be introduced only when not doing so would be even costlier.

Any allowed feature is also an organizational cost to learn it and maintain it. Especially when you have a large codebase that doesn't currently use that feature. Lots of features have pitfalls (e.g., the lifetime issues here), and those can add a significant cost.

I'm not totally against lambda, but if we allow it, let's start with a safer subset of possible usage. We can grow from there once we have experience.

--
BR
Maciej Pawlowski
Opera Desktop Wroclaw

[Daniel Murphy]

I'm not totally against lambda, but if we allow it, let's start with a safer subset of possible usage. We can grow from there once we have experience.

I'm totally fine with this.  Our bind & callback system has a lot of convenience features and protections (like the auto-refcounting of scoped_refptrs) that don't have an obvious clean & low-hassle solution in lambdas, but I believe that we could figure this out given some time with the feature.  Having a restricted allowance of lambdas at first, like enforcing no capturing, give us time to get used to the feature and think about the best ways to move forward.

Also, how easy would it be to make a clang warning for passing a pointer to a scoped_refptr in a lambda?

[Peter Kasting]

On Thu, Oct 30, 2014 at 2:25 PM, Daniel Murphy <dmu...@chromium.org> wrote:

I'm not totally against lambda, but if we allow it, let's start with a safer subset of possible usage. We can grow from there once we have experience.

I'm totally fine with this.

Me too.  I would be in favor of something that basically allowed lambdas primarily for facilitating use of STL <algorithm>s.  Given things like Bind and Callback, I think that scope is sufficient for lambdas for now, and still makes them worthwhile (there's a lot of code simplification we could do using lambdas with find_if() and the like to replace hand-rolled loops today).

PK 

[Daniel Murphy]

Also, these are great TotW's for std::function and lambdas:

https://sites.google.com/a/google.com/c-common-libraries/home/c-tips-of-the-week/totw78usingstdfunctionandstdbind

https://sites.google.com/a/google.com/c-common-libraries/home/c-tips-of-the-week/tipoftheweek79capturinglocalvariablesinlambdaexpressions

[David Michael]

I realize now I didn't express myself well, given that using anything in <algorithm> would require passing a lambda as a function parameter.

I mean something more like...  "Only use lambdas in cases where they will run in the current thread of execution. For example, a lambda may be used as a functor parameter to methods or functions that will use it immediately, such as those in <algorithm>. Do not bind or store lambdas; use base::Bind and base::Callback instead, because they offer protection against a large class of object lifetime mistakes."

Happy for anybody who wants to write a better version.

[Daniel Murphy]

Michael, what about the case that Jeffrey stated above?

MyAsyncFunction(base::Bind([](RefcountedClass* self, Arg1* a1, Arg2*
a2, Arg3 a3) {
    // body
  }, this, base::Owned(local_a1), base::Unretained(local_a2), local_a3));

If you see anything inside the []s, you'd know it was against the style guide.

 

Basically, allowing the use of lambdas in the "Bind" architecture, but with no context capturing.  This will wrap the lambda in the safely of the callback which provides all of our memory management additions.  This is where I see myself using lambdas the most. 

[David Michael]

On Thu, Oct 30, 2014 at 4:03 PM, Daniel Murphy <dmu...@chromium.org> wrote:

Michael, what about the case that Jeffrey stated above?

MyAsyncFunction(base::Bind([](RefcountedClass* self, Arg1* a1, Arg2*
a2, Arg3 a3) {
    // body
  }, this, base::Owned(local_a1), base::Unretained(local_a2), local_a3));

(side note, |this| won't get automatically ref-counted here, because we're not binding with a member function. But I do think Bind will require the use of scoped_refptr<> or base::Unretained when it sees a raw ref-counted type parameter. So the example probably should have scoped_refptr<RefcountedClass>.)

 

If you see anything inside the []s, you'd know it was against the style guide.

 

Basically, allowing the use of lambdas in the "Bind" architecture, but with no context capturing.  This will wrap the lambda in the safely of the callback which provides all of our memory management additions.  This is where I see myself using lambdas the most. 

I think this is less readable than having a separately-defined function in many/most cases. If the function is going to run at a later time or in another thread, it doesn't seem unreasonable for it to be defined separately.

That's not necessarily a good argument for banning this usage; we could reasonably count on authors and reviewers to decide what's readable on a case-by-case basis. But IMHO the benefit is small enough we could say "maybe later" for this usage.

[Daniel Cheng]

​I think the lambda version is less readable. But there's another problem: base::Bind() goes to some extra lengths to make sure |this| will be valid when the callback is dispatched. Using base::Bind() with a lambda causes us to lose this enforcement.

Daniel​

[Daniel Cheng]

​The example captured this, and my point is doing so defeats the protections base::Bind() tries to put around |this|.

Daniel​

On Thu, Oct 30, 2014 at 3:51 PM, Daniel Murphy <dmu...@google.com> wrote:

"this" is not available in the lambda unless you capture it (I believe)

[Daniel Murphy]

The protections are still there.  The lambda didn't capture it.  Capturing variables refers to everything within the square brackets "[" and "]".  In that example, "this" is being sent to "Bind", which correctly protects it in the callback object, as it is a pointer to a refcounted object.  The lambda is accepting it as a function parameter as a pointer, which is the same as if you had created the function elsewhere.

[Daniel Cheng]

I don't follow. base::Bind() makes sure that the |this| argument to a member function callback is one of the following:

1. Refcountable, so a refcount is retained by base::Callback.

2. Ownership is passed to base::Callback via base::Owned().

3. A weak pointer, so base::Callback::Run() becomes a no-op if the |this| is deleted.

4. base::Unretained(), which is more dangerous, because you must guarantee lifetime of |this| yourself

The base::Bind() call in the example doesn't bind to a member function pointer, so in fact, the behavior we end up with is #4.

Daniel

[Daniel Cheng]

Also, I just noticed that the provided example used a (presumably) refcounted type, so I guess the generic 'refcountable arguments must be bound as a scoped_refptr' logic should save you here. But there are also many instances where base::Bind() is used with member functions on non-refcounted classes, and using the lambda form would default to the dangerous behavior for those instances.

Daniel

[Daniel Murphy]

Ah, I see.  Yes, there would not be detection on that.

[Colin Blundell]

On Fri Oct 31 2014 at 2:20:19 AM Daniel Cheng <dch...@chromium.org> wrote:

Also, I just noticed that the provided example used a (presumably) refcounted type, so I guess the generic 'refcountable arguments must be bound as a scoped_refptr' logic should save you here. But there are also many instances where base::Bind() is used with member functions on non-refcounted classes, and using the lambda form would default to the dangerous behavior for those instances.

I'm confused by your saying "default to the dangerous behavior" here. If the type of |this| wasn't refcounted, my (possibly incorrect) understanding is that the example given would fail to compile, and the developer would have to choose explicitly to use base::Unretained() to get the behavior you're describing.

FWIW, I also find the lambda version less readable.

[Daniel Cheng]

​Because base::Bind() is not dealing with a member function callback (a lambda is not a member function), none of the usual checks that base::Bind() applies to |this| would be used.

Daniel​

[Colin Blundell]

On Fri Oct 31 2014 at 8:43:27 AM Daniel Cheng <dch...@chromium.org> wrote:

Because base::Bind() is not dealing with a member function callback (a lambda is not a member function), none of the usual checks that base::Bind() applies to |this| would be used.

base::Bind() applies those checks to all pointer arguments. I just confirmed that the following fails to compile:

void Baz(Foo* foo) {

}

void Foo::Bar() {

   BrowserThread::PostTask(<usual_args>, base::Bind(Baz, this));

}

 

with the expected error:

../../base/bind.h:216:3: error: static_assert failed "p3_is_refcounted_type_and_needs_scoped_refptr"

  COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P3>::value

[Daniel Cheng]

The ref counting heuristic is the only one applied to all arguments. You can bind a raw pointer to a non-refcounted class as non-this argument without any special annotations.

Daniel

[Colin Blundell]

On Fri Oct 31 2014 at 9:15:24 AM Daniel Cheng <dch...@chromium.org> wrote:

The ref counting heuristic is the only one applied to all arguments. You can bind a raw pointer to a non-refcounted class as non-this argument without any special annotations.

I see. I had inadvertently chosen a Foo that happened to be a ref-counted class for my quick-and-dirty test.

Perhaps I'm still confused, but doesn't this fact imply that in Jeffrey's example, the compiler would in fact allow |local_a1| to be passed bare (and additionally |local_a2| if |Arg2| was not a ref-counted class)? I had been under the impression that the point of that example was to show a mechanism that *eliminated* the ability to pass those pointers raw.

[Jeffrey Yasskin]

On Fri, Oct 31, 2014 at 1:36 AM, Colin Blundell <blun...@chromium.org> wrote:
>
>
> On Fri Oct 31 2014 at 9:15:24 AM Daniel Cheng <dch...@chromium.org> wrote:
>>
>> The ref counting heuristic is the only one applied to all arguments. You
>> can bind a raw pointer to a non-refcounted class as non-this argument
>> without any special annotations.
>>
>
> I see. I had inadvertently chosen a Foo that happened to be a ref-counted
> class for my quick-and-dirty test.
>
> Perhaps I'm still confused, but doesn't this fact imply that in Jeffrey's
> example, the compiler would in fact allow |local_a1| to be passed bare (and
> additionally |local_a2| if |Arg2| was not a ref-counted class)? I had been
> under the impression that the point of that example was to show a mechanism
> that *eliminated* the ability to pass those pointers raw.

That was my goal, but base::Bind()'s protection is inconsistent, so I
always forget what exactly it does. Why are non-this pointer arguments
considered less dangerous again?

[David Michael]

There *is* protection built in to Bind to make sure you don't bind a raw pointer to a ref-counted type. See here:

https://code.google.com/p/chromium/codesearch#chromium/src/base/bind.h&l=154

Here's a little demonstration CL that shows the compile error:

https://codereview.chromium.org/698493003/diff/1/base/bind_unittest.cc

"In file included from ../../base/bind_unittest.cc:5:

../../base/bind.h:154:3: error: static_assert failed "p1_is_refcounted_type_and_needs_scoped_refptr""

Note I just used global functions instead of lambdas, because it seems we'd need to do a little bit of work to make Bind work with lambdas if we wanted to use them that way. But Bind's protections should be the same in either case; they're based on parameter type.

Here's the build output with the error:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel/builds/5043/steps/compile%20%28with%20patch%29/logs/stdio

So I think Jeffrey's usage is safe. My primary objection to it is readability. I could still see it as a "maybe later" option, once we have experience with lambdas for use in algorithms.

[Daniel Murphy]

(sorry for the duplicate email guys, I keep forgetting that I can't respond to these with my google account)

I think the advantage for readability w/ lambdas in Bind comes from having all of your logic in one spot.  Files like this:

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/service_worker/service_worker_storage.cc&q=service_Worker_storage&l=5

become confusing very quickly, as they need to have a Did* method for everything. Some of the 'binded' methods make sense if they're referenced in multiple places, but the vast majority of these are just one-off 'Did*' methods that do some transforms or validations before continuing the callback chain.  The logic here would be much easier to trace if they were embedded lambdas in the calling method.

The header here explodes as well.

[Daniel Cheng]

Jeffrey's particular example is safe, but I'm talking about |this| pointers for non-refcounted classes.

class Example {

  void Run();
};

Example e;

base::Closure c = base::Bind(&Example::Run, &e);

does not compile, because base::Bind() wants you to explicitly transfer ownership, pass a weak pointer, or explicitly declare that you'll make sure |this| is valid at the time the callback runs.

Daniel

[Colin Blundell]

On Fri Oct 31 2014 at 6:16:37 PM David Michael <dmic...@chromium.org> wrote:

There *is* protection built in to Bind to make sure you don't bind a raw pointer to a ref-counted type. See here:

https://code.google.com/p/chromium/codesearch#chromium/src/base/bind.h&l=154

Here's a little demonstration CL that shows the compile error:

https://codereview.chromium.org/698493003/diff/1/base/bind_unittest.cc

"In file included from ../../base/bind_unittest.cc:5:

../../base/bind.h:154:3: error: static_assert failed "p1_is_refcounted_type_and_needs_scoped_refptr""

Note I just used global functions instead of lambdas, because it seems we'd need to do a little bit of work to make Bind work with lambdas if we wanted to use them that way. But Bind's protections should be the same in either case; they're based on parameter type.

Here's the build output with the error:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel/builds/5043/steps/compile%20%28with%20patch%29/logs/stdio

Yep, agreed; this is effectively the same experiment that I reported earlier in this thread. My comment is about the usage of base::Owned on a1 in his example. My understanding is that that example would compile just fine passing in the a1 instance raw (and also the a2 instance raw if that isn't on a refcounted type, which was ambiguous in his example).

[David Michael]

On Fri, Oct 31, 2014 at 12:47 PM, Colin Blundell <blun...@chromium.org> wrote:

On Fri Oct 31 2014 at 6:16:37 PM David Michael <dmic...@chromium.org> wrote:

There *is* protection built in to Bind to make sure you don't bind a raw pointer to a ref-counted type. See here:

https://code.google.com/p/chromium/codesearch#chromium/src/base/bind.h&l=154

Here's a little demonstration CL that shows the compile error:

https://codereview.chromium.org/698493003/diff/1/base/bind_unittest.cc

"In file included from ../../base/bind_unittest.cc:5:

../../base/bind.h:154:3: error: static_assert failed "p1_is_refcounted_type_and_needs_scoped_refptr""

Note I just used global functions instead of lambdas, because it seems we'd need to do a little bit of work to make Bind work with lambdas if we wanted to use them that way. But Bind's protections should be the same in either case; they're based on parameter type.

Here's the build output with the error:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel/builds/5043/steps/compile%20%28with%20patch%29/logs/stdio

Yep, agreed; this is effectively the same experiment that I reported earlier in this thread. My comment is about the usage of base::Owned on a1 in his example. My understanding is that that example would compile just fine passing in the a1 instance raw (and also the a2 instance raw if that isn't on a refcounted type, which was ambiguous in his example).

Okay, sorry for missing the point earlier. I tried tweaking my example, and I see what you mean. Bind happily accepts raw pointer parameters for parameters with non-refcounted type. (I didn't realize that, and I'm with Jeffrey in wondering why we make the distinction between |this| and other parameters.)

So lambdas plus base::Bind is currently less safe than current base::Bind usage even in Jeff's example.

In that case, I'm even more convinced we should *at most* allow lambdas in limited contexts, e.g. for <algorithm>.

[Colin Blundell]

On Fri Oct 31 2014 at 8:28:04 PM David Michael <dmic...@chromium.org> wrote:

On Fri, Oct 31, 2014 at 12:47 PM, Colin Blundell <blun...@chromium.org> wrote:

On Fri Oct 31 2014 at 6:16:37 PM David Michael <dmic...@chromium.org> wrote:

There *is* protection built in to Bind to make sure you don't bind a raw pointer to a ref-counted type. See here:

https://code.google.com/p/chromium/codesearch#chromium/src/base/bind.h&l=154

Here's a little demonstration CL that shows the compile error:

https://codereview.chromium.org/698493003/diff/1/base/bind_unittest.cc

"In file included from ../../base/bind_unittest.cc:5:

../../base/bind.h:154:3: error: static_assert failed "p1_is_refcounted_type_and_needs_scoped_refptr""

Note I just used global functions instead of lambdas, because it seems we'd need to do a little bit of work to make Bind work with lambdas if we wanted to use them that way. But Bind's protections should be the same in either case; they're based on parameter type.

Here's the build output with the error:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel/builds/5043/steps/compile%20%28with%20patch%29/logs/stdio

Yep, agreed; this is effectively the same experiment that I reported earlier in this thread. My comment is about the usage of base::Owned on a1 in his example. My understanding is that that example would compile just fine passing in the a1 instance raw (and also the a2 instance raw if that isn't on a refcounted type, which was ambiguous in his example).

Okay, sorry for missing the point earlier. I tried tweaking my example, and I see what you mean. Bind happily accepts raw pointer parameters for parameters with non-refcounted type. (I didn't realize that, and I'm with Jeffrey in wondering why we make the distinction between |this| and other parameters.)

+1 to your parenthetical comment; I was surprised by this as well.

[Hendrik]

Can we please add lambdas now?  I'm reviewing code that will benefit from them.

Thanks!

[Nico Weber]

I think this thread is waiting for an example where they'd be beneficial. So far, there's only examples of using them with Bind(), where the consensus seems to be that they're both less safe and less readable.

On Mon, Nov 3, 2014 at 3:01 PM, Hendrik <hend...@chromium.org> wrote:

Can we please add lambdas now?  I'm reviewing code that will benefit from them.

Thanks!

--

[Peter Kasting]

On Mon, Nov 3, 2014 at 3:12 PM, Nico Weber <tha...@chromium.org> wrote:

I think this thread is waiting for an example where they'd be beneficial. So far, there's only examples of using them with Bind(), where the consensus seems to be that they're both less safe and less readable.

I think there's been several cases of people opining that lambdas would be quite useful with e.g. STL <algorithm>s.  I assume you don't need illustrative examples of that?

To my mind what's missing here is a clear way of stating the proposed rules for lambdas such that we avoid whatever dangerous cases exist with e.g. Bind().  Should we be saying "don't use lambdas with Bind()"? Or should the rule be narrower or broader than that?

PK

[Alex Vakulenko]

Right now you can't use lambdas with Bind unless one provides a running adapter to marry the two. You can, however, use lambdas that don't capture anything, since they decay to simple C-style function pointers.

I think the rule could be, use lambdas where appropriate (similar to what official Google style guide says). If we don't provide binding adapters for functors, the problem of abusing lambdas with capture in context of base::Callback is a non-issue.

--

[David Michael]

I made an attempt at a rule earlier:

"Only use lambdas in cases where they will run in the current thread of execution. For example, a lambda may be used as a functor parameter to methods or functions that will use it immediately, such as those in <algorithm>. Do not bind or store lambdas; use base::Bind and base::Callback instead, because they offer protection against a large class of object lifetime mistakes."

I'm sure it could be worded better.

[Peter Kasting]

On Mon, Nov 3, 2014 at 3:29 PM, David Michael <dmic...@chromium.org> wrote:

I made an attempt at a rule earlier:
"Only use lambdas in cases where they will run in the current thread of execution. For example, a lambda may be used as a functor parameter to methods or functions that will use it immediately, such as those in <algorithm>. Do not bind or store lambdas; use base::Bind and base::Callback instead, because they offer protection against a large class of object lifetime mistakes."

Coupled with the restrictions on capturing that the Google style guide already supplies, this seems good to me.  (Sorry for not noticing it before, my eyes glazed over after 57 messages about Bind() safety.)  Other people want to weigh in on this proposal?

PK 

[Nico Weber]

+1, together with something like "no default capture, no ownership transfer via capture" this sounds good to me.

[Alex Vakulenko]

+1

[Peter Kasting]

"No default captures" is in the Google style guide.  Is "No ownership transfer via capture" what we want?  I feel like I could imagine a lambda taking a scoped_ptr (maybe for algorithms that act on containers of scoped_ptrs?).

PK

[Alex Vakulenko]

Yes, I think we are talking about ownership transfer via capture. At this point we cannot capture move-only types anyway (until we adopt C++14), so scoped_ptr can't be used in lambda capture. The rule should regarding transferring ownership implicitly by capturing a raw pointer.

[Peter Kasting]

On Mon, Nov 3, 2014 at 4:05 PM, Alex Vakulenko <avaku...@chromium.org> wrote:

Yes, I think we are talking about ownership transfer via capture. At this point we cannot capture move-only types anyway (until we adopt C++14), so scoped_ptr can't be used in lambda capture. The rule should regarding transferring ownership implicitly by capturing a raw pointer.

Can you give an example of what you'd like to prevent?  I'm trying to think about e.g. for_each() on a vector of raw pointers and I don't think I'm grasping what kinds of cases are bad.

PK 

[Alex Vakulenko]

If you use for_each on a vector of raw pointers, then you will be passing those pointers to the parameters of the lambda. We are talking about capturing other pointers.

So this is fine:

std::vector<Data*> data;

std::for_each(data.begin(), data.end(), [](Data* p) { p->DoSomething(); });

But this one is not:

void DoSomething(Data* data) {

  //...

}

template<typename F>

void Invoke(const F& func) {

  func();

}

void foo() {

  Data* some_data = new Data();

  auto my_lambda = [some_data]() { DoSomething(some_data); delete some_data; };

  Invoke(my_lambda);

}

Here the captured raw pointer |some_data| will be freed as part of lambda execution sequence. So, we are capturing the raw pointer and effectively transferring the ownership of the object to the lambda...

[Peter Kasting]

On Mon, Nov 3, 2014 at 4:25 PM, Alex Vakulenko <avaku...@chromium.org> wrote:

If you use for_each on a vector of raw pointers, then you will be passing those pointers to the parameters of the lambda. We are talking about capturing other pointers.

So this is fine:

std::vector<Data*> data;

std::for_each(data.begin(), data.end(), [](Data* p) { p->DoSomething(); });

But this one is not:

void DoSomething(Data* data) {

  //...

}

template<typename F>

void Invoke(const F& func) {

  func();

}

void foo() {

  Data* some_data = new Data();

  auto my_lambda = [some_data]() { DoSomething(some_data); delete some_data; };

  Invoke(my_lambda);

}

Here the captured raw pointer |some_data| will be freed as part of lambda execution sequence. So, we are capturing the raw pointer and effectively transferring the ownership of the object to the lambda...

What about:

void TakeOwnership(scoped_ptr<Foo> f);

{

  vector<Foo*> foos;

  foos.push_back(new Foo());

  std::for_each(foos.begin(), foos.end(), [](Foo* f) { TakeOwnership(make_scoped_ptr(f)); });

}

Doesn't this effectively transfer ownership of the object to the lambda (which then transfers it out via a function call)?

I guess I'm still totally mystified on whet sort of general pattern we're trying to prevent here.  Sorry for denseness :(

PK

[Alex Vakulenko]

Yes, sure, your last example looks as nasty. However, I'd say it is "pushing it". Your use case has no semantics specific to lambdas. It will happen with a normal function call or a callback. And base::Bind() wouldn't save you in this case either. So, I wouldn't bother mentioning this specific case in the rules...

[Peter Kasting]

On Mon, Nov 3, 2014 at 4:34 PM, Alex Vakulenko <avaku...@chromium.org> wrote:

Yes, sure, your last example looks as nasty. However, I'd say it is "pushing it". Your use case has no semantics specific to lambdas. It will happen with a normal function call or a callback. And base::Bind() wouldn't save you in this case either. So, I wouldn't bother mentioning this specific case in the rules...

So, pardon my continued denseness, but are you moving away from the position that we should say something about captures that "transfer ownership"? Or are we still supposed to have some rule about this?  If the latter, I still have no idea what the rule is trying to do.

I really need this spelled out as if I am utterly clueless, because I am.

PK

[Nico Weber]

I was thinking of something like

void foo() {

  scoped_ptr<Foo> p(new Foo);

  //subtly_takes_ownership(p.Pass(), [p]() { });

      // Drat! Doesn't build! Let's fix this!:

  Foo* pf = p.get();

  subtly_takes_ownership(p.Pass(), [pf]() { /* whoops, pf in invalid here */ });

}

…but I suppose that's no different than being silly with raw pointers in other places.

[Peter Kasting]

Yeah, I think that's probably equally hazardous with lambdas and without.

I propose we stick with David's proposed text + the Google style guide.

PK

[Krzysztof Olczyk]

On Tue, Nov 4, 2014 at 12:27 AM, Alex Vakulenko <avaku...@chromium.org> wrote:

Right now you can't use lambdas with Bind unless one provides a running adapter to marry the two. You can, however, use lambdas that don't capture anything, since they decay to simple C-style function pointers.

The lambdas that don't capture anything will neither work our of the box, I think,

because they will not implicitly get converted while matching the specialization of RunnableAdapter<>. They would require an explicit static_cast to function pointer type.

Regarding the topic of this thread itself, David's proposal also sounds good to me

(with extra style restrictions mentioned above).

-- 

Best regards,

Krzysztof Olczyk

Opera Software ASA

TV & Devices

[Nico Weber]

This is now allowed, with roughly David's proposed wording: http://chromium-cpp.appspot.com/

[Vincent Scheib]

I see that "base::Bind([]( type param ){}, param)" has been used somewhat in the codebase, but wasn't fully approved on this thread. I'd like to see if there's enough support to clearly mark this pattern as acceptable in the styleguide, as it does help with readability instead of bouncing around to different locations in the source file, e.g. this patch, which would be establishing a pattern for many future methods. The non-capturing lambda saves duplicating all methods.

On Thursday, October 30, 2014 at 3:23:49 PM UTC-7, David Michael wrote:

On Thu, Oct 30, 2014 at 4:03 PM, Daniel Murphy <dmu...@chromium.org> wrote:

Michael, what about the case that Jeffrey stated above?

MyAsyncFunction(base::Bind([](RefcountedClass* self, Arg1* a1, Arg2*
a2, Arg3 a3) {
    // body
  }, this, base::Owned(local_a1), base::Unretained(local_a2), local_a3));

(side note, |this| won't get automatically ref-counted here, because we're not binding with a member function. But I do think Bind will require the use of scoped_refptr<> or base::Unretained when it sees a raw ref-counted type parameter. So the example probably should have scoped_refptr<RefcountedClass>.)

 

If you see anything inside the []s, you'd know it was against the style guide.

 

Basically, allowing the use of lambdas in the "Bind" architecture, but with no context capturing.  This will wrap the lambda in the safely of the callback which provides all of our memory management additions.  This is where I see myself using lambdas the most. 

I think this is less readable than having a separately-defined function in many/most cases. If the function is going to run at a later time or in another thread, it doesn't seem unreasonable for it to be defined separately.

That's not necessarily a good argument for banning this usage; we could reasonably count on authors and reviewers to decide what's readable on a case-by-case basis. But IMHO the benefit is small enough we could say "maybe later" for this usage.

 

--

[Daniel Cheng]

There was a more recent thread about captureless lambdas and base::Bind: https://groups.google.com/a/chromium.org/forum/#!topic/cxx/QxjsPELDYdQ. In short, using captureless lambdas with base::Bind is allowed.

Daniel

[Vincent Scheib]

Thanks, here's a patch to clarify the style guide:

https://codereview.chromium.org/2382103003/

[sanzoku rounin]

2014年11月5日水曜日 9時39分07秒 UTC+9 Nico Weber: