On Mon, Dec 28, 2020 at 1:44 PM connor horman via Std-Proposals <
std-proposals_at_[hidden]> wrote:

> Re: Barry Revzin's comment, your right, I should probably have iterated a
> use case.
> This was in relation to some code that entailed a checked downcast on a
> std::unique_ptr, something that is available for shared_ptr but absent in
> the case of unique_ptr. This would specifically involve
> dynamic_pointer_cast, but I decided to add the other two (notably omitting
> reinterpret_pointer_cast, because that is probably more of a footgun then
> regular reinterpret_cast, if one can believe that is possible).
>

The nice thing about dynamic_pointer_cast is that you can make it compile
only if both source and target types have virtual destructors — in which
case I *think* it's guaranteed that default_delete<Source> will do the same
thing as default_delete<Target>.

The bad thing about dynamic_pointer_cast is that it can fail at runtime.
You tried to deal with that by giving it the signature
    unique_ptr<U> dynamic_pointer_cast(unique_ptr<T>&& rhs)
and saying that it conditionally-moves-out-of `rhs`. But
conditionally-moving-out-of is a code smell.
The STL generally avoids conditionally-moving-out-of, by such gymnastics as
insert_return_type <https://en.cppreference.com/w/cpp/container/set/insert>.
What you really want to return here, maybe, is something like
`nonstd::expected<unique_ptr<U>, unique_ptr<T>>`. We expect a
unique_ptr<U>, but if there's an error, we get a unique_ptr<T> instead.

(I don't believe you can have a unique_ptr<void> but you can correct me on
> that),
>

My impression is that you can have an instance of unique_ptr<void,Deleter>
but you cannot have an instance of unique_ptr<void> ==
unique_ptr<void,default_delete<void>>.

[...] I believe it would also work with allocators, using
> dynamic_cast<void*>(p), then deallocating that (would a pseudo-destructor
> call invoke the virtual destructor in the Most-derived object?).
>

No. dynamic_cast<void*>(p) is a one-way operation, that gives you a plain
old memory address. You can't take that (void*) and make an explicit
destructor call on it; and even if you did, you'd have to name the type you
were trying to destroy, which in this case you have not got.

    Cat *p = new Tiger;
    void *vp = dynamic_cast<void*>(p);
    vp->~auto(); // syntax error
    vp->~Tiger(); // error, *vp is not a Tiger, it's void
    reinterpret_cast<Tiger*>(vp)->~Tiger(); // OK but not fit for your
purpose
    // and even in this last case, we've managed to call the destructor but
we're still leaking memory, unless we also
    ::operator delete(vp, sizeof(Tiger)); // OK but not fit for your
purpose

I'm wondering if a similar issue could be present with the shared_ptr
> overloads that already exist in the standard.
>

No, because with shared_ptr, the original deletion action is type-erased
into the shared_ptr's heap-allocated control block. There is never any need
to "rebind" or "convert" one kind of deletion action to another. The
standard static_pointer_cast operates only on the pointer value of the
shared_ptr; it doesn't mess with the deletion operation at all.

    // https://godbolt.org/z/Tef4Ko
    std::shared_ptr<std::vector<Tiger>> vs =
std::make_shared<std::vector<Tiger>>(10, Tiger());
    std::shared_ptr<Cat> cs = std::shared_ptr<Cat>(vs, vs->at(2));
    std::shared_ptr<Tiger> ts1 = std::shared_ptr<Tiger>(cs,
static_cast<Tiger*>(cs.get()));
    std::shared_ptr<Tiger> ts2 = std::static_pointer_cast<Tiger>(cs);

When the last of vs, cs, ts1, and ts2 goes out of scope, the heap-allocated
vector<Tiger> is destroyed using the destructor for vector<Tiger>.


> <https://godbolt.org/z/Y376Kr>
>>
>> In real code, I'd want to see a function named clearly after its purpose,
>> e.g. `ptr_downcast` if you want to use it for downcasts within a hierarchy:
>> https://godbolt.org/z/abG33b
>> If I saw someone trying to do this in the presence of a custom deleter,
>> and/or via a generically named function like `static_pointer_cast`, I'd bet
>> that they didn't know what they were doing.
>>
> Perhaps, though, as mentioned, these are existing functions in the
> standard, so the name have precedent, I'm just adding overloads for the
> unique_ptr case.
>

Yes, but your new functions behave fundamentally differently and (IMO) are
generally unsafer. So if I saw someone using them by the name
static_pointer_cast, as if they *behaved* like static_pointer_cast, I'd
assume that person didn't know what they were doing, and I'd want to audit
that code very carefully.

HTH,
Arthur