make_shared and inaccessible constructors

[Andrew Schepler]

When a class is intended to be used only from std::shared_ptr, for instance when using std::enable_shared_from_this, it can be desirable to enforce this by making constructors and destructors private or protected.  A working example:

class SharedClass :

    public std::enable_shared_from_this<SharedClass>

{

public:

    static std::shared_ptr<SharedClass> create()

    { return { new SharedClass, [](SharedClass* p) { delete p; } }; }

private:

    SharedClass() = default;

    ~SharedClass() = default;

    SharedClass(const SharedClass&) = delete;

    SharedClass& operator=(const SharedClass&) = delete;

};

However, this example misses out on the benefits of std::make_shared, like doing just one allocation for both the control block and the pointed-to object. We might like to rewrite the body of create() as { return std::make_shared<SharedClass>(); }, but this is invalid as the required constructor and destructor are inaccessible.  This can't be solved with a friend declaration.  Although the Standard says make_shared will evaluate ::new(pv) T(forward<Args>(args)...), exactly what class or function will use that expression is an implementation-specific detail.

Now, we can borrow an idea from boost::iterator_core_access.  The documentation for class template boost::iterator_facade recommends users implement the interface functions such as dereference() and increment() as private, and add "friend class boost::iterator_core_access;".  iterator_core_access itself merely wraps these interface functions and exposes them to the actual various implementation classes that require them.

So I propose a new class std::shared_ptr_access with no public or protected members, and to specify that for std::make_shared<T>(args...) and std::alloc_shared<T>(a, args...), the expressions ::new(pv) T(forward<Args>(args)...) and ptr->~T() must be well-formed in the context of std::shared_ptr_access.

An implementation of std::shared_ptr_access might look like

namespace std {

    class shared_ptr_access

    {

        template <typename _T, typename ... _Args>

        static _T* __construct(void* __pv, _Args&& ... __args)

        { return ::new(__pv) _T(forward<_Args>(__args)...); }

        template <typename _T>

        static void __destroy(_T* __ptr) { __ptr->~_T(); }

        template <typename _T, typename _A>

        friend class __shared_ptr_storage;

    };

}

where __shared_ptr_storage<T,A> is a hypothetical implementation detail that requires access to shared_ptr_access::__construct and shared_ptr_access::__destroy.  Multiple friend declarations in shared_ptr_access may be appropriate.

[Nicol Bolas]

On Tuesday, May 2, 2017 at 5:25:24 PM UTC-4, Andrew Schepler wrote:

When a class is intended to be used only from std::shared_ptr, for instance when using std::enable_shared_from_this, it can be desirable to enforce this by making constructors and destructors private or protected.  A working example:

class SharedClass :

    public std::enable_shared_from_this<SharedClass>

{

public:

    static std::shared_ptr<SharedClass> create()

    { return { new SharedClass, [](SharedClass* p) { delete p; } }; }

private:

    SharedClass() = default;

    ~SharedClass() = default;

    SharedClass(const SharedClass&) = delete;

    SharedClass& operator=(const SharedClass&) = delete;

};

However, this example misses out on the benefits of std::make_shared, like doing just one allocation for both the control block and the pointed-to object. We might like to rewrite the body of create() as { return std::make_shared<SharedClass>(); }, but this is invalid as the required constructor and destructor are inaccessible.  This can't be solved with a friend declaration.

One solution to this problem is to use a key type. This is a type which is move/copyable, but whose default constructor is private. The key type is made a friend of `create`, and your `SharedClass`'s (now public) constructors all take this type as a parameter. This allows `create` to create an instance of the key type, thus allowing it to construct `SharedClass` instances.

And since this form of friendship is done through the type system, `make_shared<SharedClass>` has the power to create such types, so long as the caller can pass along an instance of the key type.

[dalb...@gmail.com]

I know that this thread is one year old, but I'd just like to say that I find this proposal by Andrew extremely useful and elegant. The alternatives are:

- Simply keep the constructors public, and rely on the developers not to call them. Possibly error-prone on big projects.

- Keep the constructors public but using a PassKey idiom as suggested by Nicol. This prevents developer mistakes, but still clutters the public interface with what is supposed to be an implementation detail (the constructors are still publicly visible on Doxygen, etc.)

- Make the constructors private, and in the implementation of SharedClass::create() use an "enable_make_shared" nested class. This keeps the public interface clean but seems an abuse of inheritance and in theory requires the base class to have a virtual destructor.

For more details, many variants of these approaches are documented here: https://stackoverflow.com/questions/8147027/how-do-i-call-stdmake-shared-on-a-class-with-only-protected-or-private-const

Instead, the proposal of Andrew seems to have none of the drawbacks, and perfectly matches the desired semantics, which is: "our class should only be created via make_shared", that is, the constructors of the class should be private, and the class should befriend whoever make_shared delegates for the construction.

[Glen Fernandes]

On Tuesday, July 10, 2018 at 8:24:49 AM UTC-4, dalb...@gmail.com wrote:

I know that this thread is one year old, but I'd just like to say that I find this proposal by Andrew extremely useful and elegant. The alternatives are:

- Simply keep the constructors public, and rely on the developers not to call them. Possibly error-prone on big projects.

- Keep the constructors public but using a PassKey idiom as suggested by Nicol. This prevents developer mistakes, but still clutters the public interface with what is supposed to be an implementation detail (the constructors are still publicly visible on Doxygen, etc.)

- Make the constructors private, and in the implementation of SharedClass::create() use an "enable_make_shared" nested class. This keeps the public interface clean but seems an abuse of inheritance and in theory requires the base class to have a virtual destructor.

 

There's also:

- Keep the constructors private, but leverage an Allocator that can construct and use std::allocate_shared instead of std::make_shared.

LWG2070 has been resolved, std::allocate_shared is required to use std::allocator_traits<Allocator>::construct instead of placement new.

e.g. Using his SharedClass example above, this works for the create() function:

class SharedClass {

public:

    static std::shared_ptr<SharedClass> create() {

        struct A : std::allocator<SharedClass> {

            void construct(void* p) { ::new(p) SharedClass(); }

            void destroy(SharedClass* p) { p->~SharedClass(); }

        };

        return std::allocate_shared<SharedClass>(A());

    }

private:

    SharedClass() = default;

    ~SharedClass() = default;

};

And doesn't involve more than one allocation.

Glen

[Glen Fernandes]

I wrote:

e.g. Using his SharedClass example above, this works for the create() function:

I remembered how we actually specified things in P0674R1, so the following is probably a better example instead: 

 

class SharedClass {

public:

    static std::shared_ptr<SharedClass> create()

    { return std::allocate_shared<SharedClass>(A<SharedClass>()); }

private:

    template<class T> struct A : std::allocator<T> {

        void construct(void* p) { ::new(p) SharedClass(); }

        void destroy(SharedClass* p) { p->~SharedClass(); }

    };

    SharedClass() = default;

    ~SharedClass() = default;

};

i.e. We mandated rebind before construct. 

In any case, not many lines of code to achieve the same thing (private constructors, single allocation shared_ptr).

Glen

[Nicol Bolas]

On Thursday, July 12, 2018 at 9:41:58 AM UTC-4, Glen Fernandes wrote:

In any case, not many lines of code to achieve the same thing (private constructors, single allocation shared_ptr).

A key type is just as easy to use, and doesn't require everyone to use your specialized allocator type. There are advantages to being able to use `std::allocator`.

[Glen Fernandes]

A key type would be easier to use. 

There are advantages to both, depending on what the user needs: For example, if the user wanted SharedClass to be Trivial, they wouldn't be able to do that by providing a SharedClass constructor that accepted an argument of a key type.  While SharedClass() = default; with a custom allocator and std::allocate_shared would meet that requirement.

Glen

[Nicol Bolas]

What good is that? If the default constructor isn't public, then nobody can actually trivially construct one. And if you use some creation function or even `allocator::construct` to create it, it will almost certainly perform value initialization, not leave the type uninitialized.

So why would you need a type to be Trivial with a private default constructor?

Also, if you're using a custom allocator, types like `vector` which may have optimized copy/move behavior for trivially copyable types cannot use it, since they don't know if your `construct/destruct` methods call `new/delete` or not. So there is an inherent disadvantage to using an allocator to control just construction/destruction.

[Glen Fernandes]

On Thursday, July 12, 2018 at 11:24:45 AM UTC-4, Nicol Bolas wrote:

On Thursday, July 12, 2018 at 10:34:56 AM UTC-4, Glen Fernandes wrote:

A key type would be easier to use. 

There are advantages to both, depending on what the user needs: For example, if the user wanted SharedClass to be Trivial, they wouldn't be able to do that by providing a SharedClass constructor that accepted an argument of a key type.  While SharedClass() = default; with a custom allocator and std::allocate_shared would meet that requirement.

What good is that? If the default constructor isn't public, then nobody can actually trivially construct one.

Certain C++ library implementations (including C++ standard library implementations) special case trivial types. For example: libstdc++ will only use __builtin_memmove in algorithms like std::copy when T is trivial (not just if T is trivially-copy-assignable). In such cases, std::copy on SharedClass* defined with a defaulted constructor that preserves triviality could benefit from that.

And if you use some creation function or even `allocator::construct` to create it, it will almost certainly perform value initialization, not leave the type uninitialized.

If someone wanted that SharedClass::create() creation function to perform default-initialization instead of value-initialization, using std::allocate_shared with a custom allocator that constructs using default-initialization is also one way of achieving that.

Glen

[Nicol Bolas]

On Thursday, July 12, 2018 at 12:06:25 PM UTC-4, Glen Fernandes wrote:

On Thursday, July 12, 2018 at 11:24:45 AM UTC-4, Nicol Bolas wrote:

On Thursday, July 12, 2018 at 10:34:56 AM UTC-4, Glen Fernandes wrote:

A key type would be easier to use. 

There are advantages to both, depending on what the user needs: For example, if the user wanted SharedClass to be Trivial, they wouldn't be able to do that by providing a SharedClass constructor that accepted an argument of a key type.  While SharedClass() = default; with a custom allocator and std::allocate_shared would meet that requirement.

What good is that? If the default constructor isn't public, then nobody can actually trivially construct one.

Certain C++ library implementations (including C++ standard library implementations) special case trivial types. For example: libstdc++ will only use __builtin_memmove in algorithms like std::copy when T is trivial (not just if T is trivially-copy-assignable).

It should be based on TriviallyCopyable, not Trivial. As I understand it, that's how it works in MSVC's standard library. In any case, that's the only limitation on performing such byte-wise copy operations that the standard imposes.

In such cases, std::copy on SharedClass* defined with a defaulted constructor that preserves triviality could benefit from that.

You can still use a key rather than an allocator. Just make your default constructor private, and all those optimizations come back. After all, it's not like `std::copy` is going to call the default constructor.

My point is that you shouldn't be using an allocator for this. The reason is what I outlined: the kind of optimization you're talking about cannot be done in `std::vector` if you are using a custom allocator that has its own `construct/destruct` functions.

[Glen Fernandes]

On Thursday, July 12, 2018 at 12:51:42 PM UTC-4, Nicol Bolas wrote:

It should be based on TriviallyCopyable, not Trivial. As I understand it, that's how it works in MSVC's standard library. 

Agreed: With something like std::copy specifically, using TriviallyCopyable to choose __builtin_memmove is fine, provided that it also requires that T is Assignable (i.e. As long as std::copy with a TriviallyCopyable but Non-assignable T fails to compile.).

libc++ just uses IsTriviallyCopyAssignable to select __builtin_memmove, which covers the above.

Glen

[Nicol Bolas]

Actually, it doesn't. `is_trivially_copy_assignable` is a weaker requirement than `is_trivially_copyable`. The former asks only if the copy assignment operator is trivial. But that's not sufficient to allow bytewise copies to work; the standard is very clear on this. To do a bytewise assignment, you need TriviallyCopyable as well.

[Andrew Schepler]

I don't agree that the key workaround is just as easy as the allocator one.  The key way gets rather messy when you want the ability to copy the object and/or for the class to be a polymorphic base class.  If we want both, things start looking like this:

class SharedBase
    : public std::enable_shared_from_this<SharedBase>
{
protected:
    class CtorKey {
         CtorKey() = default;
         friend SharedBase;
    };

public:
    SharedBase(const SharedBase&) = delete;
    SharedBase& operator=(const SharedBase&) = delete;
    virtual ~SharedBase() = default;

    template <typename... Arg>
    static auto create(Arg&& ... arg)
        -> std::enable_if_t<std::is_constructible_v<SharedBase, CtorKey, Arg...>,
            std::shared_ptr<SharedBase>>
    { return std::make_shared<SharedBase>(ctor_key(), std::forward<Arg>(arg)...); }

    virtual std::shared_ptr<SharedBase> clone() const
    { return create(*this); }

    // Effectively protected:
    explicit SharedBase(CtorKey) : m_a(), m_b(0) {}
    SharedBase(CtorKey, const SharedBase& src) : m_a(src.m_a), m_b(src.m_b) {}
    SharedBase(CtorKey, std::string a, int b=0) : m_a(std::move(a)), m_b(b) {}

protected:
    static CtorKey ctor_key() { return {}; }

private:
    std::string m_a;
    int m_b;
};

class SharedDerived final
    : public SharedBase
{
public:
    template <typename... Arg>
    static auto create(Arg&& ... arg)
        -> std::enable_if_t<std::is_constructible_v<SharedDerived, CtorKey, Arg...>,
            std::shared_ptr<SharedDerived>>
    { return std::make_shared<SharedDerived>(ctor_key(), std::forward<Arg>(arg)...); }

    std::shared_ptr<SharedBase> clone() const override
    { return create(*this); }

    // Effectively private (?):
    explicit SharedDerived(CtorKey key) : SharedBase(key), m_c(), m_d(0) {}
    SharedDerived(CtorKey key, std::string a, int b, std::string c, int d)
        : SharedBase(key, std::move(a), b), m_c(std::move(c)), m_d(d) {}

private:
    std::string m_c;
    int m_d;
};

In any use of the key pattern, there's extra implementation details appearing in a "public" section, which could be confusing to someone who just wants to use the class, and will also show up when using Doxygen or similar tools.  If we want a copyable class with several members, we must write a pseudo copy constructor and get all the member copies right - we can't use "= default".  If we want to allow derived classes, each derived class needs to duplicate the key logic, meaning the author needs to somewhat understand it; in my opinion the key pattern is somewhat less obvious than the allocator pattern.  And then we're counting on authors of derived classes not to misuse the key to incorrectly create base, derived, or even objects of other derived types, or to break the key type's encapsulation.

So I like the allocator pattern better than the key pattern. It might have its own small cost if an implementation uses extra memory to store a copy of the allocator but avoids that when using make_shared. But it looks like at least the g++ library uses the empty base optimization whenever the allocator type is empty and not final, and so there would be no extra memory use with Glen's SharedClass::A example.

The friend proposal would make things even simpler than the allocator pattern, but I'm pretty satisfied with just recommending the allocator pattern for this sort of problem.

[Andrew Schepler]

And see, I even forgot the pseudo copy constructor SharedDerived(CtorKey, const SharedDerived&);

[Glen Fernandes]

On Thursday, July 12, 2018 at 7:31:42 PM UTC-4, Andrew Schepler wrote:

I don't agree that the key workaround is just as easy as the allocator one.  The key way gets rather messy when you want the ability to copy the object and/or for the class to be a polymorphic base class.  If we want both, things start looking like this:

[...]

So I like the allocator pattern better than the key pattern.

[...]

The friend proposal would make things even simpler than the allocator pattern, but I'm pretty satisfied with just recommending the allocator pattern for this sort of problem.

You're not alone. While I have no strong preference, I do remember that private constructors with make_shared was one of smaller the motivating reasons for supporting LWG 2070 (i.e. allocate_shared using Allocator construct for construction).

We finally resolved[1] LWG 2070 with our P0674R1 paper, so that behavior is in for C++2a but library implementations already support it today.

(We had already implemented[2] it in Boost in 2014; Boost 1.58 shipped with a boost::allocate_shared that supported the above behavior).

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0674r1.html

[2] https://www.boost.org/doc/libs/1_67_0/libs/smart_ptr/doc/html/smart_ptr.html#history

Glen

[Boris Dalstein]

Thanks a lot Glen for the idea of using std::allocate_shared instead of
std::make_shared! After a bit of experimenting, I do find it to be my
preferred workaround, for the same reasons highlighted by Andrew.

Yet, I still think that the proposed std::shared_ptr_access would be
much cleaner. In my opinion, the current need to use this allocator
pattern (or alternatives) is one of the few annoyances which still makes
using shared_ptr a pain compared to raw pointers or custom-made smart
pointers, in many common idioms. In fact, this issue alone makes me
consider switching my whole project from using shared_ptr to using
custom intrusive reference counting smart pointers, a tempting
alternative which I've been trying hard to resist so far ("keep it
simple", etc.). Ideally, I do think the C++ landscape is better off if
most libraries use standard smart pointers instead of custom-made ones,
and I believe that additional facilities such as the proposed
std::shared_ptr_access, by elegantly solving one of the well-known
limitations of make_shared, makes this ideal landscape more likely to
happen.

If it makes sense to other people, and if a C++ expert is available to
guide me a little (I'm far from a C++ expert myself), I would be more
than willing to spend the time writing an actual proposal for this.

- Boris