7

To be specific: direct-list-initialization (cppreference.com (3)).

Both std::make_shared and uniform initialization features were introduced in C++11. So we can use aggregate initialization when allocating objects on heap: new Foo{1, "2", 3.0f}. This is a nice way to directly initialize objects that have no constructors, such as aggregates, pods, etc.

A real-life scenarios, such as declaring casual structures within a function, to efficiently supply set of arguments to a lambda became very common, in my experience:

void foo()
{
    struct LambdaArgs
    {
        std::string arg1;
        std::string arg2;
        std::string arg3;
    };

    auto args = std::make_shared<LambdaArgs>(LambdaArgs{"1", "2", "3"});

    auto lambda = [args] {
        /// ...
    };

    /// Use lambda
    /// ...
}

Here auto args = std::make_shared<LambdaArgs>("1", "2", "3"); whould be nice but isn't going to work, because std::make_shared is usually implemented as:

template<typename T, typename... Args>
std::shared_ptr<T> make_shared(Args && ...args)
{
    return std::shared_ptr<T>(new T(std::forward<Args>(args)...));
}

So we're stuck with the auto args = std::make_shared<LambdaArgs>(LambdaArgs{"1", "2", "3"});.

The problem that was supposed to be solved with std::make_shared still persists for object without constructor. And the workaround is not only unaesthetic but also less efficient.

Is this another oversight or are there some reasons that defend this choice. Specifically, what pitfalls can be in the list initialization solution? std::make_unique was introduced later, in C++14, why does it too follow same pattern?

GreenScape
  • 7,191
  • 2
  • 34
  • 64
  • 1
    Not an answer to your question, but a workaround: If you use a tuple instead of a casual struct, you will be provided with the appropriate constructor, and it's even terser: `using LambdaArgs = std::tuple`; –  Jan 04 '17 at 22:02
  • The problem `std::make_shared` is solving is making sure everything is cleaned up properly if an exception is thrown after the object is created but before the shared_ptr takes ownership. With that said this does look like a minor oversight. – Kevin Jan 04 '17 at 22:04
  • You may find this interesting: http://stackoverflow.com/questions/24234480/stdmake-shared-with-stdinitializer-list – marcinj Jan 04 '17 at 22:20
  • Am I missing something - why not simply create `make_shared` that is of your interest [like this](http://melpon.org/wandbox/permlink/sTBVNPq6uNxymcuK)? – W.F. Jan 04 '17 at 22:33
  • 2
    @W.F: You may be missing that `T{a,b}` and `T(a,b)` can have different effects. E.g. they have different effects for `T` = `std::string`. Ideally there should be a curly braces based `make_shared` *in addition to* the old one, either with slightly different names or with some disambiguating dummy argument (tag). – Cheers and hth. - Alf Jan 05 '17 at 01:32
  • "*A real-life scenarios, such as declaring casual structures within a function, to efficiently supply set of arguments to a lambda became very common, in my experience:*" Why is heap-allocating storage for a lambda a common thing for you? Storing those arguments directly would be more efficient. Well, perhaps not if you've got dozens of copies of a lambda running around. But in that case... why do you have dozens of copies of it running around? – Nicol Bolas Jan 05 '17 at 02:29
  • @NicolBolas Because C++11 does not allow you to move arguments into lambda. So, for example, if you have a *heavy* stuff, you can't pass it like this `auto lambda = [heavy_stuff] {... };` without copying it. So, `std::shared_ptr` is basically the only feasible solution. – GreenScape Jan 05 '17 at 06:12
  • @W.F. The question was about possible pitfalls of having your own version, and *Alf* actually points to one of them. – GreenScape Jan 05 '17 at 06:13
  • @GreenScape: "*So, std::shared_ptr is basically the only feasible solution.*" Here's another feasible solution: don't use a lambda. If your lambda really does need "heavy stuff", then it isn't a lambda. It's a struct with an `operator()` overload. And since you're *already defining* a struct... just give it an `operator()` overload and put the body of your lambda in it. Problem solved. – Nicol Bolas Jan 05 '17 at 16:31
  • @Nicol Bolas you're definitely haven't heard about `boost::asio`, have you? Lambda is lambda, it isn't made to serve a specific task, it's made to serve you, your needs. – GreenScape Jan 05 '17 at 16:36
  • @GreenScape: I don't see what that has to do with what I said. I outlined a mechanism for you to avoid the overhead of copying "heavy stuff" by using a struct rather than a lambda+`shared_ptr`. What does `asio` have to do with that? – Nicol Bolas Jan 05 '17 at 16:40
  • @NicolBolas you've said *If your lambda really does need "heavy stuff", then it isn't a lambda*. – GreenScape Jan 05 '17 at 16:42

2 Answers2

8

Specifically, what pitfalls can be in the list initialization solution?

All of the typical pitfalls of using list-initialization.

For example, the hiding of non-initializer_list constructors. What does make_shared<vector<int>>(5, 2) do? If your answer is "constructs an array of 5 ints", that's absolute correct... so long as make_shared isn't using list-initialization. Because that changes the moment you do.

Note that suddenly changing this would break existing code, since right now all of the indirect initialization functions use constructor syntax. So you can't just change it willy-nilly and expect the world to keep working.

Plus one more unique to this case: the narrowing issue:

struct Agg
{
  char c;
  int i;
};

You can do Agg a{5, 1020}; to initialize this aggregate. But you could never do make_shared<Agg>(5, 1020). Why? Because the compiler can guarantee that the literal 5can be converted to a char with no loss of data. However, when you use indirect initialization like this, the literal 5 is template-deduced as int. And the compiler cannot guarantee that any int can be converted to a char with no loss of data. This is called a "narrowing conversion" and is expressly forbidden in list initialization.

You would need to explicitly convert that 5 to a char.

The standard library has an issue on this: LWG 2089. Though technically this issue talks about allocator::construct, it should equally apply to all indirect initialization functions like make_X and C++17's in-place constructors for any/optional/variant.

why does it too follow same pattern?

It follows the same pattern because having two different functions that look almost identical that have radically and unexpectedly different behaviors would not be a good thing.

Note that C++20 resolves the aggregate part of this issue at least by making constructor-style syntax invoke aggregate initialization if the initializers would have been ill-formed for regular direct initialization. So if T is some aggregate type (with no user-declared constructors), and T(args) wouldn't invoke a copy/move constructor (the only constructors that take arguments which a type with no user-declared constructors could have), then the arguments will instead be used to attempt to aggregate initialize the structure.

Since allocator::construct and other forms of forwarded initialization default to direct-initialization, this will let you initialize aggregates through forwarded initialization.

You still can't do other list-initialization stuff without explicitly using an initializer_list at the call site. But that's probably for the best.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • BTW casual structure is not the substitute for *aggregate*, it's a name that I came up with for structures that are declared for one-time use within a narrow context (*body of a function*). – GreenScape Jan 05 '17 at 06:31
  • The sad part is that there seems to be no solution in sight to this problem, not even in principle. no? – alfC Jun 12 '18 at 22:33
  • @alfC: [P0960](http://wg21.link/P0960) would solve this problem adequately. – Nicol Bolas Jun 12 '18 at 22:44
  • @NicolBolas, ok, thanks for the link. But how will variantions of `std::make_unique>(3,4)` can be interpreted either as `std::vector{3, 4}` and `std::vector(3,4)`? – alfC Jun 12 '18 at 23:02
  • @alfC: I said that it would solve *this problem* adequately: the initialization of an aggregate. There's no generalized solution for using list initialization, because the entire premise is contingent on an incorrect assumption: that a list of values is sufficient information to know how the user wants to use them to initialize any object. – Nicol Bolas Jun 13 '18 at 00:41
1

The problem that was supposed to be solved with std::make_shared still persists for object without constructor.

No, the problem does not persist. The main problem make_shared is solving is a potential for a memory leak between the object is allocated and the ownership is taken by the smart pointer. It is also capable of removing one extra allocation for control block.

Yes, it is inconvenient to not be able to use a direct initialization, but this was never the declared goal of make_shared.

SergeyA
  • 61,605
  • 5
  • 78
  • 137
  • 1
    make_shared has nothing to do with memory leaks. What make_shared provides is guaranteed locality of reference between the reference count and object, potentially avoiding an extra cache miss when dereferencing it. –  Jan 04 '17 at 22:08
  • 1
    @Frank, perhaps you can educate yourself by looking here: http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared – SergeyA Jan 04 '17 at 22:09
  • 1
    I stand corrected. Thanks for the reference! Edit: Your link does say that the avoiding the extra allocation is the main reason, and avoiding the memory leak is a nice benefit, I don't feel THAT foolish. –  Jan 04 '17 at 22:09
  • @Frank, it doesn't actually say what is more important, it just lists them in a specific order, but in my view, program correctness always trumps optimizations. – SergeyA Jan 04 '17 at 22:44
  • The "It is also" is the main and sole motivation for `make_shared`. The memory leak problem was a main motivation for `make_unique` (Herb offered a GOTW-like debate on his site about it). – Cheers and hth. - Alf Jan 05 '17 at 01:10
  • 1
    `make_shared` came from Boost, and [the Boost docs](http://www.boost.org/doc/libs/1_63_0/libs/smart_ptr/make_shared_array.html) say "Originally the Boost function templates make_shared and allocate_shared were for efficient allocation of shared objects only.". Kneel and surrender! Basta! – Cheers and hth. - Alf Jan 05 '17 at 01:17
  • 1
    **−1** “The main problem make_shared is solving is a potential for a memory leak” is at least historically incorrect. And it's worth noting that it doesn't solve the memory leak problem: it merely *supports* writing safe code. In order to solve the problem so that client code using a function taking shared pointers, is constrained to safe ways (not multiple `new` expressions in the arguments), one needs a formal argument type that can not be initialized with pointer or temporary `shared_ptr`. – Cheers and hth. - Alf Jan 05 '17 at 01:26
  • I don't see how it solves anything. Memory leak is obviously caused by the bad way of coding. Yes, scenario described on *cppreference* is covered but it does not mean that coder can't make more mistakes. Moreover, it never was an unavoidable problem to the extent where fix is needed in the standard library. Just allocate smart pointer before passing it to the function, problem solved. – GreenScape Jan 05 '17 at 06:23