Code review comment for lp:~pete-woods/unity-api/add-glib-assigner-class

Revision history for this message
Michi Henning (michihenning) wrote :

> I've put together the sort of thing I was talking about now. This includes the
> same sort of thing for both char array stuff and GObject instances.
>
> If you can figure out a way to do the Glib side with derived templates I'm all
> ears. I struggled very muchly with the template instantiation order learning
> some new things on the way when I just tried to make the GlibAssigner a
> template instead of macro funkiness.

I'll have a look tomorrow and see whether I can come up with something.

> In terms of the deletion being surprising, let's give an example, assuming
> we've got a wrapper type that auto converts to **:
>
> void do_stuff(Thingy** t) {}
>
> auto t = unique_glib(new_thingy());
> do_stuff(t);
> // t is now empty!
>
> I don't like that, and it really does feel like the same problem as the old
> auto_ptrs:
>
> void do_stuff(auto_ptr<Thingy> t) {}
>
> auto_ptr<Thingy> t(new Thingy());
> do_stuff(t);
> // t is now empty!

Not sure there is anything surprising here at all. It's exactly the same as passing any value by non-const reference. When the call returns, the value may or may not have changed. I expect that: after all, I'm passing the value so it *can* be changed by the called function.

> I also don't think you're putting across a concrete argument about why the
> "Assigners" are bad. Is it just because there's more typing? Can you express
> more clearly what the problem is, please.

It feels a bit clunky to have to explicitly instantiate an assigner instance just so I can pass the error instance. I have to do this each and every time, and there is no other use case for the Assigner class, as far as I can see. The assigners aren't bad, but the usage looks a bit inelegant to me. Ideally, I'd like to have a type that is a straight drop-in replacement for GError and can be passed wherever GError** is expected by a glib function, without any extra noise.

> I prefer them because the "wacky types that do magic stuff" only live very
> briefly for the duration of a function call. Instead of having smart pointers
> with unusual properties that live potentially a long time.

The way I wrote AutoGError, I don't think there is any magic stuff. It behaves like a value type. The only thing I can do is ask it whether the value exists (operator bool()), and access the value (via operator-> and operator*). If there is no value, and I try to use it, I get undefined behavior, just as I do with unique_ptr. If I pass it "by reference" where a glib function expects a GError "by reference", I'm not going to be surprised if an old error is replaced with a newer one if I reuse the same error instance.

In essence, this boils down to the question "should out params be syntactically distinguishable at the point of call?" In C++, the answer is usually "no" because, when a parameter can be changed by a called function, it normally is passed by non-const reference. In C#, you have to explicitly decorate the parameter at the point of call instead. (I do have sympathy for both approaches.)

« Back to merge proposal