On 9/26/24 4:41 AM, Giuseppe D'Angelo wrote:
Hi,
Thank you for the review.
Il 25/09/24 20:13, Jason Merrill ha scritto:
Supporting the ambiguous case seems pointless to me but, that is what
the accepted proposal specifies, so indeed that's what we should
implement.
It's fundamentally the same with is_base_of, also supporting ambiguous
and unaccesible bases.
Ah, good point.
+/* Nonzero iff TYPE is derived virtually from PARENT. Ignores
accessibility and
+ ambiguity issues. */
+#define DERIVED_VIRTUALLY_FROM_P(PARENT, TYPE) \
+ (lookup_base ((TYPE), (PARENT), ba_require_virtual, NULL,
tf_none) != NULL_TREE)
I don't think we need a macro for this.
I've added it to match the existing DERIVED_FROM_P macro, but can
certainly remove it, since it's got only one user. Should I remove it or
keep it for symmetry?
Remove it, thanks, I don't think any other users are likely.
+ ba_require_virtual = 1 << 3 /* Require a virtual base */
Comment should end with period and two spaces.
+ /* Skip this result if we require virtual inheritance
+ and this is not a virtual base. */
Likewise.
Will fix.
- data.want_any = access == ba_any;
+ data.want_any = (access & ~ba_require_virtual) == ba_any;
data.offset = offset;
It seems like you aren't using ba_require_virtual|ba_any, so do you need
to mess with bitwise &?
base_access_flags is meant to be a bitmask, but for some reason ba_any
is 0, and I didn't want to change the semantics of that == check.
My point is that you can leave the == check alone without changing its
semantics, as ba_any is still used only by itself.
With your patch you set want_any when called with ba_require_virtual,
which seems unnecessary; the want_any behavior doesn't do anything for
this case.
+@defbuiltin{bool __builtin_is_virtual_base_of (@var{base_type},
@var{derived_type})}
+If @var{base_type} is a virtual base class of @var{derived_type}
+([class.derived]) then the trait is @code{true}, otherwise it is
@code{false}.
+Top-level cv-qualifications of @var{base_type} and
+@var{derived_type} are ignored.
We might mention here that this ignores access and ambiguity.
Will add. This was adapted from __is_base_of's docs; do you want me to
add the same remark there?
Sure, thanks.
Jason
