std::enable_shared_from_this: Is public inheritance necessary?

[peter.ste...@gmail.com]

I had a problem with my implementation of the <memory> standard header. 

std::enable_shared_from_this only works when inherited publicly. This is not in the standard.

Standard:

23.11.2.5 Class template enable_shared_from_this [util.smartptr.enab]

1 A class T can inherit from enable_shared_from_this<T> to inherit the shared_from_this member functions

that obtain a shared_ptr instance pointing to *this.

Is my implementation wrong, or should the standard be more explicit.

The problem was even worse, because there was no compile time error.

[Ville Voutilainen]

[util.smartptr.shared.const]/1:

"In the constructor definitions below, enables shared_from_this with
p, for a pointer p of type Y*, means
that if Y has an unambiguous and accessible base class that is a
specialization of enable_shared_from_-
this (23.11.2.5)"

The base class enable_shared_from_this must be accessible, and the
standard says so.

[Ville Voutilainen]

And this was made specified by
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0033r1.html,
prior to that paper the standard was anything but clear about it.

[Jonathan Wakely]

On Monday, 15 May 2017 15:45:18 UTC+1, peter.ste...@gmail.com wrote:

I had a problem with my implementation of the <memory> standard header. 

std::enable_shared_from_this only works when inherited publicly. This is not in the standard.

Standard:

23.11.2.5 Class template enable_shared_from_this [util.smartptr.enab]

1 A class T can inherit from enable_shared_from_this<T> to inherit the shared_from_this member functions

that obtain a shared_ptr instance pointing to *this.

Is my implementation wrong, or should the standard be more explicit.

The intended behaviour of enable_shared_from_this was clarified by http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0033r1.html 

That makes it clear that the "enables shared_from_this with p" behaviour only happens if the base class is accessible and unambiguous.

 

The problem was even worse, because there was no compile time error.

Which is the right behaviour. If the enable_shared_from_this base is not accessible then it can't be used, but that shouldn't mean the code can't be compiled.

[Jonathan Wakely]

On Monday, 15 May 2017 15:45:18 UTC+1, peter.ste...@gmail.com wrote:

I had a problem with my implementation of the <memory> standard header. 

std::enable_shared_from_this only works when inherited publicly. This is not in the standard.

Standard:

23.11.2.5 Class template enable_shared_from_this [util.smartptr.enab]

1 A class T can inherit from enable_shared_from_this<T> to inherit the shared_from_this member functions

that obtain a shared_ptr instance pointing to *this.

Is my implementation wrong, or should the standard be more explicit.

I would argue the standard is entirely correct. You can inherit it to to inherit the member functions. They might not be accessible, but they're still inherited. So [util.smartptr.enb] is correct.

If the inheritance is not public and unambiguous then the weak_ptr member of the enable_shared_from_this base class isn't initialized, but that's covered by the [util.smartptr.shared.const] wording Ville quoted.

N.B. http://en.cppreference.com/w/cpp/memory/enable_shared_from_this already says you need to publicly inherit.

[peter.ste...@gmail.com]

Thanks for the help. Seems like my implementation and the standard are correct and I am wrong.

Still I am thinking that the behaviour is somewhat suprising. The Golden Rule of interface design (Make your interface easy to use correctly and hard to use incorectly) is violated.

If some compiler writer is reading this, please make the compiler generate a warning if someone

tries to inherit privately or protected from enable_shared_from_this.

[peter.ste...@gmail.com]

I played around with that problem a little more. I was able to make private inheritance of std::enable_shared_from_this working, by making it accessable through friendship. 

I wonder if the standard was phrased to make this use case work too.

Sadly, because friendship isnt inherited (for good reasons), i do not believe, that it is possible to make private inheritance of enable_shared_from working generally through friendship.

Is there a way to detect privately inherited classes of a type?

[Paul "TBBle" Hampson]

 How did you get it working by friendship?  We've just bounced off this in a project we're working on, and I was considering proposing an extension to enable_shared_from_this to allow enabling the internal weak_ptr (or equivalent) subsequently.

Since the code that evaluates the setting of "weak_this" isn't required to be inline in the shared_ptr constructor, I don't think there's a definite thing you can name in the friend declaration.

Alternatively, [util.smartptr.shared.const]/1 says that the constructor evaluates the statement, and hence you can simply friend all the constructors (and the relevant make_shared). In which case I suspect all of these implementations are wrong:

* libc++: std::shared_ptr::__enable_weak_this

** This does the conversion in the constructors, but SFINAEs the enabling away in std::shared_ptr::__enable_weak_this, where it's too late. (Partly my fault: https://bugs.llvm.org//show_bug.cgi?id=32979)

* libstdc++: std::enable_shared_from_this::__enable_shared_from_this_base does the conversion, called from std::shared_ptr::_M_enable_shared_from_this_with itself called from the constructors

* MSVC 14.10: std::_Enable_shared does the conversion.

So it seems that if the rule was weakend slightly, to say that the constructor or other code in the relevant shared_ptr class evaluated that code, then two of three would be compliant, and private std::enable_shared_from_this would simply require friending the shared_ptr class template for your type, if you wanted your weak_this initialised.

The use-case I've bounced off this from is building an Entity-Component system. The Entities are shared_ptr, and own unique_ptr to their components. We want to be able to expose a method to get an aliased shared_ptr to a Component from the Entity, using the Entity's ref-count, without making the Entity publicly support enable_shared_from_this.

[Ville Voutilainen]

On 16 May 2017 at 12:13, Paul "TBBle" Hampson <paul.h...@pobox.com> wrote:
> Alternatively, [util.smartptr.shared.const]/1 says that the constructor
> evaluates the statement, and hence you can simply friend all the
> constructors (and the relevant make_shared). In which case I suspect all of
> these implementations are wrong:
> * libc++: std::shared_ptr::__enable_weak_this
> ** This does the conversion in the constructors, but SFINAEs the enabling
> away in std::shared_ptr::__enable_weak_this, where it's too late. (Partly my
> fault: https://bugs.llvm.org//show_bug.cgi?id=32979)
> * libstdc++: std::enable_shared_from_this::__enable_shared_from_this_base
> does the conversion, called from
> std::shared_ptr::_M_enable_shared_from_this_with itself called from the
> constructors
> * MSVC 14.10: std::_Enable_shared does the conversion.

In general, library implementations assume they can indirect as much
as they please,
so you can't befriend library functions and make this thing work,
certainly not portably,
and certainly not when the library implementation performs the
conversion in a function
that you're not allowed to name.

> The use-case I've bounced off this from is building an Entity-Component
> system. The Entities are shared_ptr, and own unique_ptr to their components.
> We want to be able to expose a method to get an aliased shared_ptr to a
> Component from the Entity, using the Entity's ref-count, without making the
> Entity publicly support enable_shared_from_this.

That's what
template< class Y >
shared_ptr( const shared_ptr<Y>& r, element_type* ptr ) noexcept;
is for. See

https://wandbox.org/permlink/Vs8D3hLASRaYAryV

[Paul "TBBle" Hampson]

That's exactly what we've done. And making enable_shared_from_this<Component> be privately inherited breaks it, because shared_from_this() now throws bad_weak_ptr.

That's the problem we're trying to solve, that brought me to this thread. Our only option at the moment is to inline the weak_ptr in Component (effectively inlining enable_shared_from_this) and add a private method called from our static construction function, which initialises that weak_ptr.

Making this work in the standard would require (I guess, possibly there are better options) some kind of standard-named version of enable_shared_from_this::enable_weak_this, which would mean if I privately inherit from enable_shared_from_this, I can then publicly provide an enable_weak_this of my own, which'll be found and do the right thing (calling this->enable_shared_from_this::enable_weak_this).

Interestingly enough, private constructors _also_ don't work with make_shared, which is annoying but harder to solve. There, we'd have to use a private tag-struct constructor variable I guess.

[Ville Voutilainen]

On 16 May 2017 at 13:42, Paul "TBBle" Hampson <paul.h...@pobox.com> wrote:
> On Tuesday, 16 May 2017 19:39:53 UTC+10, Ville Voutilainen wrote:
>>
>> On 16 May 2017 at 12:13, Paul "TBBle" Hampson <paul.h...@pobox.com> wrote:
>>
>> > The use-case I've bounced off this from is building an Entity-Component
>> > system. The Entities are shared_ptr, and own unique_ptr to their
>> > components.
>> > We want to be able to expose a method to get an aliased shared_ptr to a
>> > Component from the Entity, using the Entity's ref-count, without making
>> > the
>> > Entity publicly support enable_shared_from_this.
>>
>> That's what
>> template< class Y >
>> shared_ptr( const shared_ptr<Y>& r, element_type* ptr ) noexcept;
>> is for. See
>>
>> https://wandbox.org/permlink/Vs8D3hLASRaYAryV
>
>
> That's exactly what we've done. And making
> enable_shared_from_this<Component> be privately inherited breaks it, because
> shared_from_this() now throws bad_weak_ptr.

Right, that constructor allows Component to return shared_ptr<Entity>
without Entity publicly supporting
enable_shared_from_this.

> That's the problem we're trying to solve, that brought me to this thread.

Okay; the problem indeed, in general, is that you can't know what
exact part of the library implementation
converts Component to enable_shared_from_this<Component>. So you can't
befriend that part, so you
must use public inheritance.

To provide a better mechanism to use enable_shared_from_this with
non-public inheritance requires
a proposal. I don't think it's a simple patch-fix. I do feel sympathy
for users wanting to do that, I do that
again and again whenever I use enable_shared_from_this, because public
inheritance is not something
I do by default. Then I realize, again, that I can't do private
inheritance in this case. I might be able to
cope with a conversion operator, but I don't want that to be public either.

[Ville Voutilainen]

On 16 May 2017 at 14:09, Ville Voutilainen <ville.vo...@gmail.com> wrote:
> inheritance in this case. I might be able to
> cope with a conversion operator, but I don't want that to be public either.

..and it doesn't work because such a conversion operator is a worse
match than a standard conversion
from derived to base.

[p_ha...@wargaming.net]

Precisely.

 

To provide a better mechanism to use enable_shared_from_this with
non-public inheritance requires
a proposal. 

Yeah, that's what I was thinking. I came here to get some feedback on potential directions such a proposal could go.

It seems to me that the best option is the one I mooted before: Have the 'activation' of enable_shared_from_this be done via calling a named method, rather than by having public inheritance from enable_shared_from_this. That way I can provide such a member without exposing the rest of the enable_shared_from_this public API. (Or even simpler, just 'using' it publicly in my derived class). It's somewhat analogous to std::size(x) (which calls x.size() if it exists, irrespective of whether it's there by inheriting from something).

The only other option that seems vaguely reasonable is to require something that I can specifically friend (std::activate_this<T>(T*) perhaps). I don't know of anything else in the standard library that works that way.

Another poor option would be providing a member function pointer somewhere (like a Deleter, as an optional parameter to the activating shared_ptr constructors). That puts the conversion on the caller, which presumably is a friend or static method of the class.

Something like shared_ptr_traits might also work, but we're verging into the truly awful here, unless there's already a good justification for such a thing, such as a desire to have shared_ptr_traits<T>::deleter_t or something.

Probably also worth fixing the text regarding 'activating' enable_shared_from_this, as it claims the constructor evaluates the example code, and that's definitely not true for any of the implementations, and since friending the constructors so mentioned doesn't give that code snippet access to the private base class, then we've failed the as-if test.

[peter.ste...@gmail.com]

So for everyone interessted:

I hacked into the <memory> header of the Dinkumware implementation for C++ Builder 10.2.

class A

 :private std::enable_shared_from_this<A>

{

public:

template<class _Ty>

friend inline void std::_Enable_shared(_Ty *_Ptr, std::_Ref_count_base *_Refptr,

typename _Ty::_EStype *);

std::shared_ptr<A> get() { return shared_from_this(); }

};

This certainly isnt portable at all and I would not use it in production code. It may break for derived classes of A. 

The std::enable_shared_from_this base class is an implementation detail and it should be possible to inherit it privately. But as it is now, you cannot.