On Tue, Jun 24, 2025 at 05:19:59PM -0400, Jason Merrill wrote:
> I think we could move the initialization of the fixed_type_p and
> virtual_access variables up, they don't need to be after cp_build_addr_expr.
I don't understand why it doesn't depend on cp_build_addr_expr.
I've tried the following patch and while it didn't regress anything on
make GXX_TESTSUITE_STDS=98,11,14,17,^C,23,26 check-g++
it regressed
FAIL: 23_containers/vector/bool/cmp_c++20.cc -std=gnu++20 (test for excess
errors)
FAIL: 23_containers/vector/bool/cmp_c++20.cc -std=gnu++26 (test for excess
errors)
In there code is PLUS_EXPR, !want_pointer, !has_empty, but uneval is true
and expr is
std::vector<bool>::begin (&c)
before cp_build_addr_expr and
&TARGET_EXPR <D.316846, std::vector<bool>::begin (&c)>
after it. resolves_to_fixed_type_p (expr) is 0 before cp_build_addr_expr
and 1 after it. v_binfo is false though, so in that
particular case I think we don't actually care about fixed_type_p value,
but it doesn't raise confidence that testing resolves_to_fixed_type_p
early is ok.
--- gcc/cp/class.cc.jj 2025-06-18 17:24:03.973867379 +0200
+++ gcc/cp/class.cc 2025-06-25 08:01:06.824278658 +0200
@@ -347,9 +347,19 @@ build_base_path (enum tree_code code,
|| processing_template_decl
|| in_template_context);
+ int nonnull_copy = nonnull;
+ fixed_type_p = resolves_to_fixed_type_p (expr, &nonnull);
+
+ /* Do we need to look in the vtable for the real offset? */
+ virtual_access = (v_binfo && fixed_type_p <= 0);
+
/* For a non-pointer simple base reference, express it as a COMPONENT_REF
without taking its address (and so causing lambda capture, 91933). */
- if (code == PLUS_EXPR && !v_binfo && !want_pointer && !has_empty && !uneval)
+ if (code == PLUS_EXPR
+ && !want_pointer
+ && !has_empty
+ && !uneval
+ && (!v_binfo || !virtual_access))
return build_simple_base_path (expr, binfo);
if (!want_pointer)
@@ -361,8 +371,10 @@ build_base_path (enum tree_code code,
else
expr = mark_rvalue_use (expr);
+ gcc_assert (resolves_to_fixed_type_p (expr, &nonnull_copy)
+ == fixed_type_p && nonnull_copy == nonnull);
+
offset = BINFO_OFFSET (binfo);
- fixed_type_p = resolves_to_fixed_type_p (expr, &nonnull);
target_type = code == PLUS_EXPR ? BINFO_TYPE (binfo) : BINFO_TYPE (d_binfo);
/* TARGET_TYPE has been extracted from BINFO, and, is therefore always
cv-unqualified. Extract the cv-qualifiers from EXPR so that the
@@ -371,9 +383,6 @@ build_base_path (enum tree_code code,
(target_type, cp_type_quals (TREE_TYPE (TREE_TYPE (expr))));
ptr_target_type = build_pointer_type (target_type);
- /* Do we need to look in the vtable for the real offset? */
- virtual_access = (v_binfo && fixed_type_p <= 0);
-
/* Don't bother with the calculations inside sizeof; they'll ICE if the
source type is incomplete and the pointer value doesn't matter. In a
template (even in instantiate_non_dependent_expr), we don't have vtables
> I think -1 doesn't distinguish between single or multiple virtual
> derivation, so handling -1 in that block might mean succeeding for a
> multiple derivation case where it ought to fail.
Ok, will keep it as is then.
> > So, shall I e.g. for the if (TREE_PRIVATE case if the outer type has
> > CLASSTYPE_VBASECLASSES walk the
> > for (vbase = TYPE_BINFO (t); vbase; vbase = TREE_CHAIN (vbase))
> > if (BINFO_VIRTUAL_P (vbase) && !BINFO_PRIMARY_P (vbase))
> > and in that case try to compare byte_position (TREE_OPERAND (path, 1))
> > against BINFO_OFFSET (vbase) and if it matches (plus perhaps some type
> > check?) then decide based on BINFO_BASE_ACCESS or something like that
> > whether it was a private/protected vs. public virtual base?
>
> It seems simpler to pass an accurate access to the build_base_field above.
> At least whether the whole BINFO_INHERITANCE_CHAIN is public or not, I
> suppose the distinction between private and protected doesn't matter.
I'm afraid I'm quite lost on what actually is public base class
that [expr.dynamic.cast] talks about in the case of virtual bases because
a virtual base can appear many times among the bases and if it is virtual
in all cases, there is just one copy of it and it can be public in some
paths and private/protected in others. And where to find that information.
I've tried the following testcase and it seems that it succeeds unless
-DP1 -DP2
-DP1 -DP3
-DP1 -DP6
-DP2 -DP3 -DP6
-DP4 -DP5 -DP6
-DP2 -DP3 -DP4 -DP5
is a subset of the -DPN options or in case of clang++ also
-DP2 -DP4 -DP5 (for that g++ passes, clang++ fails).
E.g. what is the difference between -DP1 which works and
S is private in one case and public in 2 others, while -DP1 -DP2
doesn't work and is private in two cases and public in one.
#ifdef P1
#undef P1
#define P1 private
#else
#define P1
#endif
#ifdef P2
#undef P2
#define P2 private
#else
#define P2
#endif
#ifdef P3
#undef P3
#define P3 private
#else
#define P3
#endif
#ifdef P4
#undef P4
#define P4 private
#else
#define P4
#endif
#ifdef P5
#undef P5
#define P5 private
#else
#define P5
#endif
#ifdef P6
#undef P6
#define P6 private
#else
#define P6
#endif
struct S { int a, b; virtual int bar (int) { return 0; } };
struct T : virtual P1 S { int c, d; };
struct U : virtual P2 S, virtual P3 T { int e; };
struct V : virtual P4 S, virtual P5 T, virtual P6 U { int f; S &foo () { return
(S &)*this; } };
int
main ()
{
V t;
t.f = 1;
// t.c = 2;
dynamic_cast<V &> (t.foo ());
dynamic_cast<T &> (t.foo ());
dynamic_cast<U &> (t.foo ());
}
I've tried to look at BINFO_INHERITANCE_CHAIN in the debugger and see
on this testcase 6 calls of
/* This virtual base is not a primary base of any class in the
hierarchy, so we have to add space for it. */
next_field = build_base_field (rli, vbase,
access_private_node,
offsets, next_field);
(for S in T, for S and T in U and for S, T and U in V) and
vbase->binfo.inheritance->binfo.base_accesses
are in all those cases vectors of private/public in the particular
class (so P1 in the first case, P2 and P3 in the second and third
and P4, P5 and P6 in the last 3), but from that alone one can't
judge whether dynamic_cast will work or not.
What exactly does dynamic_cast look at in the vtables to decide
if it is the public or non-public case? Or could we instead of
checking for TREE_PUBLIC/TREE_PRIVATE on the DECL_FIELD_IS_BASE
FIELD_DECLs try to tf_none implicitly convert the FIELD_DECL
type to the COMPONENT_REFs first operand's type?
Jakub
