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

On 08/02/16 15:40 +0100, Murray Cumming wrote:
On Mon, 2016-02-08 at 12:40 +0000, Jonathan Wakely wrote:
You said (1) "doesn't seem much better" than use-after-move, which I
disagree with. It's infinitely better because it is not undefined and
doesn't crash :-)

Of course. I don't want code that looks wrong and crashes. Sorry for
the confusion caused by my choice of words.

 But it's not ideal because it doesn't allow rvalues
to be passed straight from factory functions to sinks. You must
an intermediate local variable, extract a raw pointer, then use
std::move to cast back to an rvalue.

Options (2) and (3) both allow rvalues to be passed straight from
factories to sinks, but allow a non-owning pointer to be obtained at
the same time.

Ah, now I see your point. Thanks.

I do find your suggested API awkward (I've renamed add_unique() to add()):

I agree, it's not ideal, so I hope there's still a better solution
waiting to be found.

 Button* b = nullptr; //I don't like this being even momentarily uninitialized. murrayc.
  c.add(pass_and_observe(button_factory("some button"), b));

However, it's just one pass_and_observe() template that works on top of the simple
add(unique_ptr<>) overloads. People could use it they wanted to, or use a get_child()
method if they preferred, trying to leave an explicit std::move() as late as possible
so they didn't need to. I hope we can improve the occasional API that makes this
difficult currently.

By the way, would you want people to use this even without a factory method? For instance:

 Button* b = nullptr; //I don't like this being even momentarily uninitialized. murrayc.

Yep, fair enough :-)

  c.add(pass_and_observe(std::make_unique<Gtk::Button>("some button"), b));

Could do. I guess it depends on how much gtkmm devs (which I'm not!)
would want to avoid using std::move() and leaving null lvalues around.
That's not necessarily a bad thing, but is a potential source of
use-after-move bugs. I've seen it suggested that any explicit use of
std::move is a code smell because of those potential bugs (I'm not
sure I entirely agree, but can see their point).

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]