On Thu, 29 Sept 2022 at 14:12, Sébastien Bini <sebastien.bini_at_[hidden]>
wrote:

> 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.
>

Still problematic:

class Greeter : private boost::noncopyable
{
public:
    std::atomic<std::string> name;
    void run() { thrd = std::jthrd([&] { std::println("Hello {}",
name.load()); }); }
private:
    std::jthread thrd;
};

Modifying `name` from user code is fine, so it's OK (if slightly odd) for
it to be public, but `decompose`ing it out is not, since then the jthread
is referring to a destroyed object. Yet `Greeter` has no special member
functions! So the real issue is that user code is affecting an object
(Greeter::thrd) to which it has no access.

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.
>

In that case, aggregate-style structured binding would likely also work.
And that's definitely the class author's fault.


> 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.
>

Yes, and that customization point (or some `get_all`,
`get<index_sequence<I...>>`, etc.) would be expected to be a member or
friend.

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.
>

Remember, the motivation is for libraries (not necessarily the Standard
Library) to be able to write decompositions for their types; in particular,
for tuple types. For example, Boost.Tuple should be able to support
relocating decomposition without any change to its current ABI or API. So
user-provided special member functions must be allowed.

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.
>

Well, the `get` protocol is for public use, and `std::decompose` for
privileged access - although it can be used on aggregate types. Also it'd
be useful for prvalue qualified member functions, etc. where previously the
"union hack" would have been necessary, e.g.:

template<class T>
T* unique_ptr<T>::release(this unique_ptr self) {
    auto const [ptr] = std::decompose<&unique_ptr::ptr_>(reloc self);
    return ptr;
}