Hello,

Il 29/09/23 15:36, Arthur O'Dwyer via Std-Proposals ha scritto:
> On Fri, Sep 29, 2023 at 5:04 AM Giuseppe D'Angelo via Std-Proposals
> <std-proposals_at_[hidden] <mailto:std-proposals_at_[hidden]>>
> wrote:
>
>
> As per subject, I'd like to propose std::is_virtual_base_of
> (complementing `is_base_of`). You can find a draft of the proposal here:
> https://isocpp.org/files/papers/D2985R0.html
> <https://isocpp.org/files/papers/D2985R0.html>
> Any feedback is welcome.
>
>
> Just nits from me. I think it looks reasonable.
> (1) Abstract: "constrain on optimize" should be "constrain or optimize"?
> or "constrain the optimization of"? or "...optimize certain operations
> when possible"?

Sorry, was a typo => or.

> (2) In the first para of section 2, rather than saying vaguely "to
> complement the existing std::is_base_of trait," I'd prefer to see the
> exact wording of the proposed trait placed right there. That way, the
> reader won't be wondering about the intent for ambiguous/private bases;
> they'll already know the destination you're trying to get to. (Full
> disclosure: I always forget whether is_base_of considers ambiguous
> bases. It does. I think this makes it unimplementable in pure C++,
> right? it requires a compiler builtin?)

There is an implementation of `is_base_of` in pure C++, courtesy of Boost:

> https://www.boost.org/doc/libs/1_83_0/boost/type_traits/is_base_and_derived.hpp

I have, well, absolutely no idea why that code works, nor if there are
shortcomings. In the paper I called is_virtual_base_of "experts-only",
but that's suffering from arrogance, since even _I_ was able to
implement it from scratch. I would never be able to come up with that
is_base_of implementation...


Anyway, good point, I've reworded the sentence.




> E.g. "We propose the addition
> of the std::is_virtual_base_of type trait to the Standard Library, to
> detect when one class is (even ambiguously or privately) a virtual base
> of another."
> (3) "non-virtual class" should be "non-virtual base". In the same
> sentence, "cf." should be "see", and "below" could be a hyperlink to
> wherever you're talking about specifically.

Done.

> (4) weak_ptr<X> should be weak_ptr<Y> to match the Standard's terminology


> (5) In section (2.1) Prior Art, am I correct that Boost's type trait
> can't deal with ambiguous bases? This would be worth mentioning, just to
> clarify that the proposed trait will differ from the Boost implementation.

That's correct. I've remarked this limitation now multiple times.


> (6) "Each choice would also likely call for a complementary type
> trait..." But you're not proposing a complementary trait, even though
> you /are/ picking one of these two choices! I think this section could
> be reworded. Basically, you're asking whether is_virtual_base_of<B,D>
> should return true when (B is a virtual base of D) AND (B is a
> nonvirtual base of D). The simple answer is "yes, it should return true
> iff (B is a virtual base of D)." We /could/ propose a complementary
> "is_nonvirtual_base_of" trait, but we have no use-case for that, so we
> don't propose it.
> (7) "Note 3: A class is never..." could say "type", instead of "class."

Being non-normative, I've left `class`, as it should be clear from the
context.


>
> One high-level concern which I /think/ is moot but it would be nice to
> say so explicitly: Conversion from D* to B* requires dereferencing B*
> when D is a virtual base of B. Is that a "when and only when"? Or are
> there any other situations where that conversion might require
> dereferencing B*, that we should be thinking about?

Do you mean "requires dereferencing" by the implementation, as opposed
as an actual dereference in the code?


> - Things that can require inspecting a vtable include dynamic_cast,
> typeid, and accesses through `int D::*` pointers-to-data-member that
> happen to point into a virtual base.

Doesn't calling a non-static non-virtual member function defined in the
virtual base also require going into the vtable, again to find where the
virtual base subobject is, and calculate `this` for the call?

> struct B { void f(); };
> struct D : ... inherit virtually from B ... {};
> void call(D &d) { d.f(); }; // "actually" ((B &)d)::B.f(), with a bit of notation abuse


I'm not exactly sure what is the point here, was it merely to enumerate
all the cases where an implementation needs to inspect an object's vtable?


> - Things that do pointer conversions include static_pointer_cast,
> dynamic_pointer_cast, etc.

Yes, but those are fine, they work on shared_ptr, not weak_ptr, and in
there you know if the pointer is dangling or not. (I'd still like to
have them on unique_ptr as well, but that's another story.)


Thank you very much for the thorough review, uploaded a new version and
upgraded to P status for the Kona mailing.

https://isocpp.org/files/papers/P2985R0.html

-- 
Giuseppe D'Angelo