Date: Thu, 29 Sep 2022 15:12:15 +0200
On Mon, Sep 26, 2022 at 7:36 PM Edward Catmur <ecatmur_at_[hidden]>
wrote:
> On Mon, 26 Sept 2022 at 15:20, Sébastien Bini <sebastien.bini_at_[hidden]>
> wrote:
>
>> I agree with the class invariant part.
>> But the no user-provided reloc, move and copy ctor clause was not
>> motivated because of the invariant, but because we wanted to be sure that
>> subobjects of a class could be independently relocated or destroyed. So at
>> minimum we must have a default destructor. And that's not enough.
>>
>> struct PainterGuard
>> {
>> Painter* _p;
>> StateGuard(Painter& p) : _p{&p} { _p->save(); }
>> ~StateGuard() { _p->restore(); }
>> };
>>
>> class PainterWithGuard
>> {
>> public:
>> PainterWithGuard(Painter p) : _p{reloc p}, _guard{_p} {}
>> PainterWithGuard() : _guard{_p} {}
>> PainterWithGuard(PainterWithGuard) /* reloc ctor */ { _guard._p =
>> &_p; }
>> public:
>> Painter _p;
>> private:
>> PainterGuard _guard;
>> };
>>
>> This is a perfectly valid class. Its default destructor works fine.
>> Unfortunately, from outside the class (with no access to private
>> data-members), doing:
>> `auto [p] = std::decompose<&PainterWithGuard::_p>(reloc
>> painterWithGuard);`
>> Will call `restore()` on a destructed/relocated object.
>>
>> Likewise doing (in the class implementation this time):
>> `auto [p, g] = std::decompose<&PainterWithGuard::_p,
>> &PainterWithGuard::_guard>(reloc painterWithGuard);`
>> Will construct a PainterGuard that points to invalid data. This one can
>> be incriminated to the class writer as it needs access to private data.
>>
>> I agree this class has a bad design, but its safe to use otherwise.
>> std::decompose shouldn't cause crashes on badly designed classes.
>>
>> IMO the no user-provided reloc, move and copy ctor clause gives us that
>> extra guarantee, that there are no relationships between subobjects and as
>> such it is safe to independently relocate or destroy them.
>>
>
> Great example, and that's the sort of thing I was trying to describe with
> my talk of private inheritance. (PainterGuard could be a private base of
> PainterWithGuard, perhaps.)
>
> But I can still write a similar class that is currently safe, has no
> user-defined special member functions but would be unsafe to decompose:
> instead of writing or deleting the relocating constructor, have
> PainterGuard inherit from boost::noncopyable. So the existence of
> user-defined special member functions is insufficient to determine whether
> a class is safe to decompose.
>
Yes. But this example could also be viewed as to give us a hint that there
must be no user-provided default constructor as well.
> Instead, consider: `std::decompose` is accessing (on behalf of its caller)
> each direct subobject (base and data member) that is returned. But it is
> also accessing the *other* direct subobjects that it does not return, in
> order to destroy them. So let's say that to call std::decompose, you must
> have access to each direct subobject, including those that you don't
> request. (Plus their relocators or destructors, respectively.)
>
> Then `auto [p] = std::decompose<&PainterWithGuard::_p>(reloc
> painterWithGuard);` would be ill-formed because in that context,
> `painterWithGuard._guard` is ill-formed.
>
> Do you think this would work?
>
But if _guard were declared as public by mistake then all those safeties
are bypassed and std::decompose will cause trouble.
This also limits the use of std::decompose to the class implementation and
friends. It is not necessarily a bad thing as it requires the owner to know
of the class internals (or at least to be in the position of knowing...).
Besides I expect the main use of std::decompose to be from the new special
get_bindings function.
In addition, now that I think of it, the no user-provided special ctors and
dtor clause is required to allow structure binding relocation from a
class-type. In auto [x, y] = reloc data, data must be a C array (no problem
there), a class-type with all data-members coming from the class-type or
the same base, or provide a get_binding function that applies recursively.
The end result of get_binding must fall into the C array case or the
class-type case. For the class-type case, we must be allowed to split the
type into individual subobjects, and have again that sort of guarantee that
subobjects can be considered individually. Hence this no user-provided
special ctors and dtor clause must apply to the class-type. This is the
case for whatever std::decompose returns, but it is worth mentioning. And
that bit makes me think that this clause should also apply to
std::decompose.
I can't feel satisfied with this solution. We end up with two ways of
decomposing: std::decompose and the get_binding function. get_binding is
still necessary as you can't access nested subobjects from std::decompose,
or even generate objects on demand when using structured bindings. And
other times get_bindings can be bypassed if the user performs structured
binding by calling std::decompose directly. I find this dual approach a bit
confusing.
wrote:
> On Mon, 26 Sept 2022 at 15:20, Sébastien Bini <sebastien.bini_at_[hidden]>
> wrote:
>
>> I agree with the class invariant part.
>> But the no user-provided reloc, move and copy ctor clause was not
>> motivated because of the invariant, but because we wanted to be sure that
>> subobjects of a class could be independently relocated or destroyed. So at
>> minimum we must have a default destructor. And that's not enough.
>>
>> struct PainterGuard
>> {
>> Painter* _p;
>> StateGuard(Painter& p) : _p{&p} { _p->save(); }
>> ~StateGuard() { _p->restore(); }
>> };
>>
>> class PainterWithGuard
>> {
>> public:
>> PainterWithGuard(Painter p) : _p{reloc p}, _guard{_p} {}
>> PainterWithGuard() : _guard{_p} {}
>> PainterWithGuard(PainterWithGuard) /* reloc ctor */ { _guard._p =
>> &_p; }
>> public:
>> Painter _p;
>> private:
>> PainterGuard _guard;
>> };
>>
>> This is a perfectly valid class. Its default destructor works fine.
>> Unfortunately, from outside the class (with no access to private
>> data-members), doing:
>> `auto [p] = std::decompose<&PainterWithGuard::_p>(reloc
>> painterWithGuard);`
>> Will call `restore()` on a destructed/relocated object.
>>
>> Likewise doing (in the class implementation this time):
>> `auto [p, g] = std::decompose<&PainterWithGuard::_p,
>> &PainterWithGuard::_guard>(reloc painterWithGuard);`
>> Will construct a PainterGuard that points to invalid data. This one can
>> be incriminated to the class writer as it needs access to private data.
>>
>> I agree this class has a bad design, but its safe to use otherwise.
>> std::decompose shouldn't cause crashes on badly designed classes.
>>
>> IMO the no user-provided reloc, move and copy ctor clause gives us that
>> extra guarantee, that there are no relationships between subobjects and as
>> such it is safe to independently relocate or destroy them.
>>
>
> Great example, and that's the sort of thing I was trying to describe with
> my talk of private inheritance. (PainterGuard could be a private base of
> PainterWithGuard, perhaps.)
>
> But I can still write a similar class that is currently safe, has no
> user-defined special member functions but would be unsafe to decompose:
> instead of writing or deleting the relocating constructor, have
> PainterGuard inherit from boost::noncopyable. So the existence of
> user-defined special member functions is insufficient to determine whether
> a class is safe to decompose.
>
Yes. But this example could also be viewed as to give us a hint that there
must be no user-provided default constructor as well.
> Instead, consider: `std::decompose` is accessing (on behalf of its caller)
> each direct subobject (base and data member) that is returned. But it is
> also accessing the *other* direct subobjects that it does not return, in
> order to destroy them. So let's say that to call std::decompose, you must
> have access to each direct subobject, including those that you don't
> request. (Plus their relocators or destructors, respectively.)
>
> Then `auto [p] = std::decompose<&PainterWithGuard::_p>(reloc
> painterWithGuard);` would be ill-formed because in that context,
> `painterWithGuard._guard` is ill-formed.
>
> Do you think this would work?
>
But if _guard were declared as public by mistake then all those safeties
are bypassed and std::decompose will cause trouble.
This also limits the use of std::decompose to the class implementation and
friends. It is not necessarily a bad thing as it requires the owner to know
of the class internals (or at least to be in the position of knowing...).
Besides I expect the main use of std::decompose to be from the new special
get_bindings function.
In addition, now that I think of it, the no user-provided special ctors and
dtor clause is required to allow structure binding relocation from a
class-type. In auto [x, y] = reloc data, data must be a C array (no problem
there), a class-type with all data-members coming from the class-type or
the same base, or provide a get_binding function that applies recursively.
The end result of get_binding must fall into the C array case or the
class-type case. For the class-type case, we must be allowed to split the
type into individual subobjects, and have again that sort of guarantee that
subobjects can be considered individually. Hence this no user-provided
special ctors and dtor clause must apply to the class-type. This is the
case for whatever std::decompose returns, but it is worth mentioning. And
that bit makes me think that this clause should also apply to
std::decompose.
I can't feel satisfied with this solution. We end up with two ways of
decomposing: std::decompose and the get_binding function. get_binding is
still necessary as you can't access nested subobjects from std::decompose,
or even generate objects on demand when using structured bindings. And
other times get_bindings can be bypassed if the user performs structured
binding by calling std::decompose directly. I find this dual approach a bit
confusing.
Received on 2022-09-29 13:12:28