[llvm-dev] enable_shared_from_this fails at runtime when inherited privately

[Christian Schneider via llvm-dev]

Am 29.08.19 um 13:44 schrieb Jonathan Wakely:
> On Thu, 29 Aug 2019 at 12:43, Jonathan Wakely wrote:
>>
>> On Thu, 29 Aug 2019 at 11:50, Christian Schneider
>> <cschneider at radiodata.biz> wrote:
>>>
>>> Am 29.08.19 um 12:07 schrieb Jonathan Wakely:
>>>> On Thu, 29 Aug 2019 at 10:15, Christian Schneider
>>>> <cschneider at radiodata.biz> wrote:
>>>>>
>>>>> Hello,
>>>>> I just discovered, that, when using enable_shared_from_this and
>>>>> inheriting it privately, this fails at runtime.
>>>>> I made a small example:
>>>>>
>>>>> #include <memory>
>>>>> #include <boost/shared_ptr.hpp>
>>>>> #include <boost/make_shared.hpp>
>>>>> #include <boost/enable_shared_from_this.hpp>
>>>>>
>>>>> #ifndef prefix
>>>>> #define prefix std
>>>>> #endif
>>>>>
>>>>> class foo:
>>>>>        prefix::enable_shared_from_this<foo>
>>>>> {
>>>>> public:
>>>>>        prefix::shared_ptr<foo> get_sptr()
>>>>>        {
>>>>>            return shared_from_this();
>>>>>        }
>>>>> };
>>>>>
>>>>> int main()
>>>>> {
>>>>>        auto a = prefix::make_shared<foo>();
>>>>>        auto b = a->get_sptr();
>>>>>        return 0;
>>>>> }
>>>>>
>>>>> This compiles fine, but throws a weak_ptr exception at runtime.
>>>>> I'm aware, that the implementation requires, that
>>>>> enable_shared_from_this needs to be publicly inherited, but as a first
>>>>> time user, I had to find this out the hard way, as documentations (I
>>>>> use, ie. cppreference.com) don't mention it, probably because it's not a
>>>>> requirement of the standard.
>>>>
>>>> It definitely is a requirement of the standard. The new wording we
>>>> added via http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0033r1.html#spec
>>>> says that the base's weak_ptr is only initialized when the base class
>>>> is "unambiguous and accessible". It doesn't say that an ambiguous or
>>>> inaccessible base class makes the program ill-formed, so we're not
>>>> allowed to reject such a program. >
>>> I see. As far as I understand, this sentence was removed:
>>> Requires: enable_shared_from_this<T> shall be an accessible base class
>>> of T. *this shall be a subobject of an object t of type T. There shall
>>> be at least one shared_ptr instance p that owns &t.
>>>
>>> As far as I read it, this required enable_shared_from_this to be public
>>> accessible.
>>
>> No. It only required it to be publicly accessible if you called
>> shared_from_this().
>>
>>> Do you know (or someone else), why it was removed?
>>
>> Yes (look at the author of the paper :-). As I wrote in that paper:
>>
>> "The proposed wording removes the preconditions on shared_from_this so
>> that it is now well-defined to call it on an object which is not owned
>> by any shared_ptr, in which case shared_from_this would throw an
>> exception."
>>
>> Previously it was undefined behaviour to call shared_from_this() if
>> the base class hadn't been initialized to share ownership with a
>> shared_ptr. That meant the following was undefined:
>>
>> #include <memory>
>> struct X : std::enable_shared_from_this<X> { };
>> int main()
>> {
>>    X x; // not owned by a shared_ptr
>>    x.shared_from_this();
>> }
>>
>> Now this program is perfectly well-defined, but it throws an
>> exception. There is no good reason to say that program has undefined
>> behaviour (which means potentially unbounded types of errors) when we
>> can just make it valid code that throws an exception when misused.
> 
> And in order to make it well-defined, we tightened up the
> specification to say exactly how and when the weak_ptr in a
> enable_shared_from_this base class gets initialized. If it's not
> possible to initialize it (e.g. because it's private) then it doesn't
> initialize it.
> 
OK, thx for clarification and insights.
Since it is a requirement from the standard, I will add a note on 
cppreference.com, so that it is clear that it needs to be public 
inherited, and it silently fails if you don't inherit public.

>>
>>> I find it a little, umm..., inconvenient, that the compiler happily
>>> accepts it when it is clear that it never ever can work...
>>
>> The code compiles and runs. It just doesn't do what you thought it
>> would do. Welcome to C++.