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

[Jonathan Wakely via llvm-dev]

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.

>
> > 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++.