Returning scoped_ptrs without Pass()

[d...@chromium.org]

Looking through some code, I ran into this:

scoped_ptr<HttpAuthHandlerRegistryFactory> HttpAuthHandlerFactory::CreateDefault(HostResolver* host_resolver) { 

    //...

    scoped_ptr<HttpAuthHandlerRegistryFactory> registry_factory =

          make_scoped_ptr(new HttpAuthHandlerRegistryFactory());

    //...

    return registry_factory;

}

(source, CL that added the change)

This compiles and runs normally, but Pass() is not used and it's also not a temporary variable.

So questions:

1. How come the compiler doesn't complain here? Using c++11 just does that?

2. Is it safe?

3. What does that means in terms of coding conventions? Should we omit Pass() if it is not needed? Or just add it all the time for consistency?

Thanks!

[Colin Blundell]

Interesting! I'm not a C++11 expert by any means, but here's my reading...

On Fri, Sep 18, 2015 at 5:05 PM d...@chromium.org <d...@chromium.org> wrote:

Looking through some code, I ran into this:

scoped_ptr<HttpAuthHandlerRegistryFactory> HttpAuthHandlerFactory::CreateDefault(HostResolver* host_resolver) { 

    //...

    scoped_ptr<HttpAuthHandlerRegistryFactory> registry_factory =

          make_scoped_ptr(new HttpAuthHandlerRegistryFactory());

    //...

    return registry_factory;

}

(source, CL that added the change)

This compiles and runs normally, but Pass() is not used and it's also not a temporary variable.

So questions:

1. How come the compiler doesn't complain here? Using c++11 just does that?

Yes, it looks like the move constructor of scoped_ptr will get invoked here:

The move constructor is called whenever an object is initialized from xvalue of the same type, which includes

• initialization, T a = std::move(b); or T a(std::move(b));, where b is of type T
• function argument passing: f(std::move(a));, where a is of type T and f is void f(T t)
• function return: return a; inside a function such as T f(), where a is of type T which has a move constructor.

from http://en.cppreference.com/w/cpp/language/move_constructor

 

2. Is it safe?

Yes (see above).

 

3. What does that means in terms of coding conventions? Should we omit Pass() if it is not needed? Or just add it all the time for consistency?

If I'm understanding correctly, it means that we no longer need to use .Pass() when returning a scoped_ptr, and we can eliminate the comments in scoped_ptr.h that say that that is necessary.

 

Thanks!

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

[Nico Weber]

Hi,

On Fri, Sep 18, 2015 at 8:04 AM, d...@chromium.org <d...@chromium.org> wrote:

Looking through some code, I ran into this:

scoped_ptr<HttpAuthHandlerRegistryFactory> HttpAuthHandlerFactory::CreateDefault(HostResolver* host_resolver) { 

    //...

    scoped_ptr<HttpAuthHandlerRegistryFactory> registry_factory =

          make_scoped_ptr(new HttpAuthHandlerRegistryFactory());

    //...

    return registry_factory;

}

(source, CL that added the change)

This compiles and runs normally, but Pass() is not used and it's also not a temporary variable.

So questions:

1. How come the compiler doesn't complain here? Using c++11 just does that?

Look up "return value optimization" (alternative spellings "rvo", "nrvo"). If RVO doesn't happen (it's not guaranteed), the compiler will implicitly call the move constructor in this case.

 

2. Is it safe?

Yes.

 

3. What does that means in terms of coding conventions? Should we omit Pass() if it is not needed? Or just add it all the time for consistency?

The thread announcing movability for scoped_refptr (a different but similar class) recommended not using Pass when it's not needed (e.g. for RVO): https://groups.google.com/a/chromium.org/d/msg/chromium-dev/Rl6si1uNNVY/DjbHWvGyD-oJ (That's because the .Pass() prevents RVO from happening.)

 

However, I think MSVC2013 has a bug and it sometimes doesn't accept omitting the .Pass() even when it's not needed: https://connect.microsoft.com/VisualStudio/feedback/details/1105046 So the guideline is "don't use it when it's not needed _and_ it passes then Windows trybots" :-)

[Nico Weber]

On Fri, Sep 18, 2015 at 8:24 AM, Nico Weber <tha...@chromium.org> wrote:

Hi,

On Fri, Sep 18, 2015 at 8:04 AM, d...@chromium.org <d...@chromium.org> wrote:

Looking through some code, I ran into this:

scoped_ptr<HttpAuthHandlerRegistryFactory> HttpAuthHandlerFactory::CreateDefault(HostResolver* host_resolver) { 

    //...

    scoped_ptr<HttpAuthHandlerRegistryFactory> registry_factory =

          make_scoped_ptr(new HttpAuthHandlerRegistryFactory());

    //...

    return registry_factory;

}

(source, CL that added the change)

This compiles and runs normally, but Pass() is not used and it's also not a temporary variable.

So questions:

1. How come the compiler doesn't complain here? Using c++11 just does that?

Look up "return value optimization" (alternative spellings "rvo", "nrvo"). If RVO doesn't happen (it's not guaranteed), the compiler will implicitly call the move constructor in this case.

I wanted to include a link to some post explaining the details, but I couldn't find one. So here's a short summary. There are two parts to this:

1. Why does this compile at all

2. Why is it preferable to omit the .Pass() (modulo MSVC bugs)

Sadly, answering 1 will reference the answer for 2, so let's answer 2 first.

For 2.: The C++ standard says (in [class.copy]p31; http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf has the complete wording: "When certain criteria are met, an implementation is allowed to omit the copy/move construction of a class

object, even if the copy/move constructor and/or destructor for the object have side effects [...] in a return statement in a function with a class return type, when the expression is the name of a [...] automatic object [...], the copy/move operation can be omitted by constructing the automatic object directly into the function’s return value". So if you say `return value;`, then the compiler can skip calling _any_ constructors. This is called RVO (or copy elision). If the return statement is `return value.Pass();`, this isn't possible. Move constructors are fast, but no constructor calls are even faster. That's why not writing .Pass() when returning locals is preferable. However, this is an optional optimization, why is the code valid if the compiler chooses not to do this optimization?

For 1.: [class.copy]p32 says: "When the criteria for elision of a copy operation are met [...], and the object to be copied is designated by an lvalue, overload resolution to select the constructor for the copy is first performed as if the object were designated by an rvalue". So if the compiler chooses not to do RVO, it will first try to copy the object out via the rvalue constructor. That's why the code is valid even if RVO isn't done.

[Jeffrey Yasskin]

On Fri, Sep 18, 2015 at 9:52 AM, Nico Weber <tha...@chromium.org> wrote:

On Fri, Sep 18, 2015 at 8:24 AM, Nico Weber <tha...@chromium.org> wrote:

Hi,

On Fri, Sep 18, 2015 at 8:04 AM, d...@chromium.org <d...@chromium.org> wrote:

Looking through some code, I ran into this:

scoped_ptr<HttpAuthHandlerRegistryFactory> HttpAuthHandlerFactory::CreateDefault(HostResolver* host_resolver) { 

    //...

    scoped_ptr<HttpAuthHandlerRegistryFactory> registry_factory =

          make_scoped_ptr(new HttpAuthHandlerRegistryFactory());

    //...

    return registry_factory;

}

(source, CL that added the change)

This compiles and runs normally, but Pass() is not used and it's also not a temporary variable.

So questions:

1. How come the compiler doesn't complain here? Using c++11 just does that?

Look up "return value optimization" (alternative spellings "rvo", "nrvo"). If RVO doesn't happen (it's not guaranteed), the compiler will implicitly call the move constructor in this case.

I wanted to include a link to some post explaining the details, but I couldn't find one. So here's a short summary. There are two parts to this:

1. Why does this compile at all

2. Why is it preferable to omit the .Pass() (modulo MSVC bugs)

Sadly, answering 1 will reference the answer for 2, so let's answer 2 first.

For 2.: The C++ standard says (in [class.copy]p31; http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf has the complete wording: "When certain criteria are met, an implementation is allowed to omit the copy/move construction of a class

object, even if the copy/move constructor and/or destructor for the object have side effects [...] in a return statement in a function with a class return type, when the expression is the name of a [...] automatic object [...], the copy/move operation can be omitted by constructing the automatic object directly into the function’s return value". So if you say `return value;`, then the compiler can skip calling _any_ constructors. This is called RVO (or copy elision). If the return statement is `return value.Pass();`, this isn't possible. Move constructors are fast, but no constructor calls are even faster. That's why not writing .Pass() when returning locals is preferable. However, this is an optional optimization, why is the code valid if the compiler chooses not to do this optimization?

For 1.: [class.copy]p32 says: "When the criteria for elision of a copy operation are met [...], and the object to be copied is designated by an lvalue, overload resolution to select the constructor for the copy is first performed as if the object were designated by an rvalue". So if the compiler chooses not to do RVO, it will first try to copy the object out via the rvalue constructor. That's why the code is valid even if RVO isn't done.

 

Elaborating a bit, RVO is only allowed if the code would have compiled without RVO. So even if your compiler RVOs everything, the move or copy constructor still has to exist and be callable at that point, in order for the code to compile. This is a big reason it's useful to add move constructors to classes even before we can use the C++11 library.

[d...@chromium.org]

Ok, I understand better now. Thanks all for your explanations!