Despite the name, perfect forwarding is not so perfect.

In the following example, a class B is constructed by passing a :

struct A { int x, y; };

struct B {
    B(A a) { /*...*/ }
};

int main() {
  B b1({1, 2});
}

Everything’s ok: {1, 2} is deduced to be the only possible option, a braced initializer for type A.

Now, if we want to create, say, a shared_ptr instead of an object on the stack, the code inside main becomes:

int main() {
  shared_ptr<B> b2(new B({1, 2}));
}

The code still works, and the deduction happens correctly. But we know the code is wrong and dangerous, because if the constructor of shared_ptr<B> throws an exception (e.g., a bad_alloc for a failed allocation of the reference counting block), the memory allocated for B will be leaked, and all the resource attached to it will remain unreleased. As aurelienrb commented below, the code above is valid and perfectly working (C++ mandates shared_ptr‘s constructor to delete the pointer if the reference counting block cannot be allocated). The real potential problem is a bit more convoluted, and happens when building the shared_ptr inside a function call (see http://stackoverflow.com/questions/19034538/why-is-there-memory-leak-while-using-shared-ptr-as-a-function-parameter)

If we studied some C++11/14 we know that the standard library provides a solution for this, make_shared, which behaves exactly like a constructor of your class.

Exactly? Well, almost.

int main() {
  shared_ptr<B> b3 = make_shared<B>({1, 2});
}

GCC:

main.cpp:15:22: error: no matching function for call to 'make_shared'
bits/shared_ptr.h:632:5: note: candidate function not viable: requires 0 arguments, but 1 was provided
make_shared(_Args&&... __args)

Why? It’s so simple, so obvious. There’s just one constructor!

Clang is slightly more verbose…

main.cpp: In function 'int main()':
main.cpp:15:43: error: too many arguments to function 'std::shared_ptr<_Tp1> std::make_shared(_Args&& ...) [with _Tp = B; _Args = {}]'
shared_ptr<B> b3 = make_shared<B>({1, 2});
^
In file included from memory:82:0,
from main.cpp:2:
shared_ptr.h:632:5: note: declared here
make_shared(_Args&&... __args)
^~~~~~~~~~~
In file included from c++allocator.h:33:0,
from bits/allocator.h:46,
from memory:63,
from main.cpp:2:
ext/new_allocator.h: In instantiation of 'void __gnu_cxx::new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = B; _Args = {}; _Tp = B]':
bits/alloc_traits.h:455:4: required from 'static void std::allocator_traits<std::allocator<_Tp1> >::construct(std::allocator_traits<std::allocator<_Tp1> >::allocator_type&, _Up*, _Args&& ...) [with _Up = B; _Args = {}; _Tp = B; std::allocator_traits<std::allocator<_Tp1> >::allocator_type = std::allocator<B>]'
bits/shared_ptr_base.h:520:39: required from 'std::_Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp>::_Sp_counted_ptr_inplace(_Alloc, _Args&& ...) [with _Args = {}; _Tp = B; _Alloc = std::allocator<B>; __gnu_cxx::_Lock_policy _Lp = (__gnu_cxx::_Lock_policy)2u]'
bits/shared_ptr_base.h:615:4: required from 'std::__shared_count<_Lp>::__shared_count(std::_Sp_make_shared_tag, _Tp*, const _Alloc&, _Args&& ...) [with _Tp = B; _Alloc = std::allocator<B>; _Args = {}; __gnu_cxx::_Lock_policy _Lp = (__gnu_cxx::_Lock_policy)2u]'
bits/shared_ptr_base.h:1100:35: required from 'std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_make_shared_tag, const _Alloc&, _Args&& ...) [with _Alloc = std::allocator<B>; _Args = {}; _Tp = B; __gnu_cxx::_Lock_policy _Lp = (__gnu_cxx::_Lock_policy)2u]'
bits/shared_ptr.h:319:64: required from 'std::shared_ptr<_Tp>::shared_ptr(std::_Sp_make_shared_tag, const _Alloc&, _Args&& ...) [with _Alloc = std::allocator<B>; _Args = {}; _Tp = B]'
bits/shared_ptr.h:619:14: required from 'std::shared_ptr<_Tp1> std::allocate_shared(const _Alloc&, _Args&& ...) [with _Tp = B; _Alloc = std::allocator<B>; _Args = {}]'
bits/shared_ptr.h:635:39: required from 'std::shared_ptr<_Tp1> std::make_shared(_Args&& ...) [with _Tp = B; _Args = {}]'
main.cpp:15:43: required from here
ext/new_allocator.h:120:4: error: no matching function for call to 'B::B()'
{ ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
main.cpp:9:5: note: candidate: B::B(A)
B(A a) { /*...*/ }
^
main.cpp:9:5: note: candidate expects 1 argument, 0 provided
main.cpp:8:8: note: candidate: constexpr B::B(const B&)
struct B {
^
main.cpp:8:8: note: candidate expects 1 argument, 0 provided
main.cpp:8:8: note: candidate: constexpr B::B(B&&)
main.cpp:8:8: note: candidate expects 1 argument, 0 provided
Compiler exited with result code 1

What a mess! It’s actually looking for an empty constructor? And then… there’re other two candidates for deduction?!?

The default copy&move constructors: let’s delete them!

struct B {
    B(A a) { /*...*/ }
    B(const B&) = delete;
    B(B &&) = delete;
};

No luck :( (basically the same errors as above)

The only viable solution is to specify the type of A during the construction:

int main() {
  shared_ptr<B> b3 = make_shared<B>(A{1, 2});
}

Apparently make_shared‘s perfect forwarding is (tautologically) perfect only when it works. When using braced initializers, types cannot be deduced and the compiler will require some hints from you to match the correct constructor, even if there’s only one viable option.

6 Comments


  1. Is it me or there’s something wrong with the syntax here?

    shared_ptr<B> b3 = make_shared<B>(A{1, 2}));
    ?

    Reply
    • Marco Foco

      It’s you :)
      Please compile with -std=c++11 or -std=c++14 :) (PS: the tag-eating monster have removed all the template parameters on your comment. I’ll try to add them back again later).

      Reply

      • I was referring to the missing open parenthesis.

        Reply
        • Marco Foco

          Well, that’s actually an extra closed parenthesis :)
          Fixed.

          Reply
  2. aurelienrb

    Hello,
    AFAIK, there’s nothing wrong with the shared_ptr example you give (unless you have a buggy STL), the C++ standard guarantee your pointer won’t leak:
    http://stackoverflow.com/questions/11922262/what-happens-if-a-shared-ptrs-constructor-fails

    (IMO, failing to allocate memory for a shared counter is a very unlikely scenario, and if it does happens, well, you are in deep trouble anyway!)

    However there’s a more serious problem that can happen when building the pointer inside a function call:
    http://stackoverflow.com/questions/19034538/why-is-there-memory-leak-while-using-shared-ptr-as-a-function-parameter

    Regards.

    Reply
    • Marco Foco

      Thank you for the clarification.
      I remembered there was a corner case when the construction failed, and I mistakenly assumed it was when the refcounting allocation was failing.
      I’ll modify the post adding your notes tomorrow!

      Reply

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.