Re: Replace Gtk::manage() with std::unique_ptr<>?

[Jonathan Wakely <gtkmm kayari org>]

On 08/02/16 00:08 +0100, Murray Cumming wrote:
> On Sun, 2016-02-07 at 22:51 +0100, Dr. Diether Knof wrote:
> > Hello,
> >
> > I think, to use std::unique_ptr and std::move instead of Gtk::manage
> > is a good
> > idea, but it is more difficult to use:
> >
> > >   auto widget = std::make_unique<Button>("some button");
> > OK
> > >   notebook.append_page(std::move(widget), "something");
> > OK, but widget does no longer point to the botton, compare with the
> > example at
> > http://en.cppreference.com/w/cpp/utility/move
> > >   notebook.child_property_tab_expand(*widget) = true;
> > Should lead to a segmentation fault.
> [snip]
>
> Yes, it does, and that's the issue here.
>
> (I've heard various discussions of what the C++ standard specifies
> about how valid or defined the moved-from object is, but I would indeed
> prefer to avoid any use after move.)

It's not complicated, but it's often explained badly, or by people who
don't know the rules.

For any type defined in the C++ standard library (unless it specifies
stronger guarantees) a moved-from object is in a *valid* but
*unspecified* state.  Valid but unspecified means it still maintains
its invariants, but you can't assume anything more about its state. So
you can't know if it's safe to call functions that have preconditions
unless you first check that the preconditions are met.

e.g. a moved-from std::string may or may not be empty, so doing s[4]
may or may not be safe. However you can use any member functions that
have no preconditions, so you can call s.size() to discover the
string's state, and then it's safe to use s[4] if size() > 4.

Some types offer stronger guarantees and std::unique_ptr is one of
them. Obviously a type that models unique ownership has to stop owning
its pointer after it transfers to another object, so std::unique_ptr
is guaranteed to be null after it is moved from. So dereferencing it
is undefined behaviour (typically resulting in a segfault).

(In general the guarantees above are only true for types defined in
the Standard Library. For non-standard types it is good style, and
highly recommended, to follow at least the "valid but unspecified"
guarantee for moved-from objects, with stronger guarantees for types
like smart pointers where that makes sense. However in general class
authors can define whatever semantics they want for their own types
after they are moved from. So if a class author says that after being
moved from her type can only be destroyed or assigned a new value,
that's OK.)

> [snip]
> > So, in order to create a widget, move it into a container and use it
> > afterwards, we have to create two pointers (the unique-pointer and a
> > normal
> > pointer):
> >
> > >   auto widget = std::make_unique<Button>("some button");
> > OK
> > >   Gtk::Widget* widget_to_expand = *widget;
> > Wrong, right is:
> > Gtk::Widget* widget_to_expand = widget.get();
> > >   notebook.append_page(std::move(widget), "something";
> > OK

This is a fairly typical idiom: get a raw (non-owning) pointer that
will remain valid after ownership is transferred from the (owning)
unique_ptr to something else.

The owning pointer is unique, but the non-owning pointer is not, you
can have as many non-owning observer pointers as you want.

The downside of this approach is that you can't do:

 extern std::unique_ptr<Button> button_factory(const char*);

 notebook.append_page( button_factory("some button"), "something" );

i.e. you can't pass an rvalue straight to a function taking a
std::unique_ptr, because you don't get a chance to take the raw
pointer from it. So you need to create an lvalue, get a raw pointer,
then cast back to an rvalue using std::move:

 auto button = button_factory("some button");
 Button* raw = button.get();
 notebook.append_page( std::move(button), "something" );
 raw->blahblah();

This is more verbose and more error-prone because 'button' is left
hanging around, containing null, tempting people to dereference it and
get a segfault.

> > Another idea is to change the add-methods to return a pointer to the
> > object
> > added. So the code can be written as (for clearity with Gtk::Button*
> > instead
> > of auto):
> > Gtk::Button* button =
> > container.add_unique(std::make_unique<Gtk::Button>("a
> > button"));
> >
> > For this, Container::add_unique() must be a (simple?) template, so it
> > returns
> > the exact type for the pointer (Gtk::Button*) and not Gtk::Widget*:
> > template<class X>
> > X* Container::add_unique(std::unique_ptr(X) widget)
> > {
> >   X* p = widget.get();
> >   this->add(std::move(widget));
> >   return p;
> > }
> > virtual void Container::add(std::unique_ptr<Gtk::Widget> widget);
> [snip]

This is nicer, because you can pass an rvalue straight in, but still
get a raw pointer:

 Button* button = c.add_unique( button_factory("some button") );

This style avoids needing any explicit std::move() to cast the
std::unique_ptr back to an rvalue, and doesn't leave any null object
hanging around.

> Yes, I've considered this, but I'm still hoping for something nicer.

The suggestion above looks fine to me. Certainly better than a
segfault :-)

> I think I'd be content with needing a call to a getter method after a
> std::move(). I think that would make sense to people.

A cleaner API would operate in terms of rvalues returned from
functions, and passed straight into other functions, avoiding
std::move entirely.

Another approach that might work as a transition from the current API
to a cleaner one would be to add:

 template<typename T>
 inline std::unique_ptr<T>
 pass_and_observe(std::unique_ptr<T> up, T*& raw)
 {
   raw = up.get();
   return up;  // N.B. 'up' is implicitly moved into return value
 }

Then you can get a non-owning pointer from an rvalue as it passes by:

 Button* b;
 c.add_unique(pass_and_observe(button_factory("some button"), b));
 b->blahblah();

This still avoids an explicit move, allowing rvalues to be returned
from sources and passed directly into sinks, but allows you to get a
non-owning observer pointer out of the rvalue as it passes by.