Re: [boost] [RFC] unique_val (handles, IDs, descriptors...)

Subject: Re: [boost] [RFC] unique_val (handles, IDs, descriptors...)
From: Miguel Ojeda (miguel.ojeda.sandonis_at_[hidden])
Date: 2018-02-15 11:18:43

On Thu, Feb 15, 2018 at 11:18 AM, Andrzej Krzemienski via Boost
<boost_at_[hidden]> wrote:
> 2018-02-14 22:18 GMT+01:00 Miguel Ojeda via Boost <boost_at_[hidden]>:
>
>> Hi all,
>>
>> While working on a medium-sized project, I had to wrap quite some
>> handles, IDs, descriptors, etc. into RAII classes. Somehow, writing
>> those and making them movable seemed more verbose than needed. I
>> searched in the STL and Boost for something already implemented, but
>> nothing seemed to come up. I decided to write my own prototype to see
>> whether I was missing something important; but actually using it in my
>> project felt quite natural and simple.
>>
>> At this point, I asked for a second opinion from a well-known expert
>> (thanks Scott!) and he agreed that it seemed like a reasonable idea
>> but, of course, he would also be surprised if others haven't come up
>> with something similar. So I polished it up a bit and uploaded it to:
>>
>>     https://github.com/ojeda/unique_val
>>
>> Below you have the introduction inlined [1].
>>
>> If you have the time, please take a look (specially to the Rationale)
>> and let me know your comments. The code itself is very short and
>> straightforward.
>>
>> If you already have this class/template in Boost somewhere that I
>> missed, please let me know! Otherwise, if you think this could be
>> interesting to put into Boost in some existing library (not sure
>> which) or as a new tiny library, I would be glad to make the effort to
>> expand it, clean it up, etc.
>>
>
> So, it is my understanding that the difference between an `int` and an
> `unique_val<int>` is that the latter is not copyable (and whoever keeps it
> as member is not copyable), and moving from it guarantees that a certain
> default value will be assigned to it. Right?

Right. Specially important is the fact that it enables those that keep
it as a member movable (if they default it -- given the current rules)
without effort, i.e. no need to implement move constructor/assignment
(which for some people is non trivial or they have not yet learned
them).

>
> If I got that right, I have two remarks.
>
> 1. In your example:
>
> ```
> class Foo
> {
>     unique_val<FooHandle> id_;
>
> public:
>     Foo() : id_(CreateFoo(/* ... */))
>     {
>         if (not id_)
>             throw std::runtime_error("CreateFoo() failed");
>     }
>
>     ~Foo()
>     {
>         if (id_)
>             DestroyFoo(id_.get());
>     }
>
>     Foo(Foo&&) = default;
>     Foo& operator=(Foo&&) = default;
> };
> ```
>
> `id_` is private (should be of no interest to the users). `Foo` is indeed
> non-copyable, but it has not been explicitly declared to the users: it has
> only been inferred from the private member (implementation detail). I would
> still expect that the class has copy operations explicitly deleted so that
> I know it is by design rather than by the current choice of private members.
>
> Your type indeed prevents accidental copying. But at least the example
> encourages a bad practice of not declaring the interface of your class
> explicitly.

You are right! The example tries to be as minimal as possible and
tries to make clear that we get the copy operations deleted by
default, but probably a comment would be nice to explicitly say to
(like the comment inside Widget), specially since it is meant for
beginners.

I partially agree with you, for some classes I typically
delete/default/implement all the special functions (basically the RAII
ones), but for the general case, it is a matter of taste: some people
prefer to be as succinct as possible (and if you are using the Rule of
Zero, that is the point).

>
> 2. Next thing I have to do after defining my `unique_value<>` type is to
> write a RAII wrapper for the resource. So, as others mentioned` maybe I
> would prefer a tool for doing the latter rather than only a movable ID.
> There is a proposal for adding this into the Standard Library:
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0052r6.pdf
>
> And also a reference implementation:
>
> https://github.com/PeterSommerlad/SC22WG21_Papers/tree/master/workspace/P0052_scope_exit/src

Very nice! Indeed, this is something that is missing from the standard
library and would be very useful.

I just took a cursory look at the proposal/code; some comments w.r.t.
unique_val (please correct me if I am wrong!)

  - it seems unique_resource is not without overhead since it has to
keep a Deleter somehow and/or know whether it has to delete or not on
the destructor. In the reference implementation a unique_resource<int>
is the size of 2 ints when using an empty Deleter (i.e. one int plus 1
byte plus the padding). unique_val can be used to implement RAII
classes without any overhead [*]
  - it does not provide for a default deleter, since it is not meant
to just hold a movable value (fine though, since it is not meant to --
same discussion with unique_handle).

Cheers,
Miguel

[*]

#include <iostream>
#include "scope_exit.h"
#include "unique_resource.h"

struct Deleter
{
    const Deleter & operator()(int) const { return *this; } }
};

int main()
{
    std::experimental::unique_resource<int, Deleter> foo(1, Deleter());
    std::cout << sizeof(foo) << '\n' << sizeof(int) << '\n';
    return 0;
}

>
> Regards,
> &rzej;
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost