Date: Fri, 29 Sep 2023 09:36:11 -0400
On Fri, Sep 29, 2023 at 5:04 AM Giuseppe D'Angelo via Std-Proposals <
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
> 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"?
(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?) 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.
(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.
(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."
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?
- 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.
- Things that do pointer conversions include static_pointer_cast,
dynamic_pointer_cast, etc.
I don't see any problematic cases in the Cartesian product of those
facilities; but I'm very unconfident that I thought of all of the things. :)
–Arthur
P.S. +1 to Jason's point; replace "causes UB" with "can segfault" or
something. :)
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
> 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"?
(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?) 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.
(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.
(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."
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?
- 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.
- Things that do pointer conversions include static_pointer_cast,
dynamic_pointer_cast, etc.
I don't see any problematic cases in the Cartesian product of those
facilities; but I'm very unconfident that I thought of all of the things. :)
–Arthur
P.S. +1 to Jason's point; replace "causes UB" with "can segfault" or
something. :)
Received on 2023-09-29 13:36:25