On Thu, 31 Jul 2025, Jason Merrill wrote:
> On 7/30/25 5:48 PM, Patrick Palka wrote:
> > On Wed, 30 Jul 2025, Patrick Palka wrote:
> >
> > > On Wed, 30 Jul 2025, Patrick Palka wrote:
> > >
> > > > On Wed, 30 Jul 2025, Patrick Palka wrote:
> > > >
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > > > > OK for trunk?
> > > > >
> > > > > -- >8 --
> > > > >
> > > > > Here the result of A::make(0, 0, 1), (0, 1, 0) and (1, 0, 0) are each
> > > > > represented as a single-element CONSTRUCTOR with
> > > > > CONSTRUCTOR_NO_CLEARING
> > > > > cleared, and we end up mangling them all as A{1}, i.e. eliding both
> > > > > the
> > > > > implicit initial and trailing zeros. Mangling them all the same seems
> > > > > clearly wrong since they're logically different values.
> > > >
> > > > Just realized that A::make(1, 0, 1) is also mangled incorrectly -- as
> > > > A{1, 1} instead of A{1, 0, 1}. So we need to consider intermediate
> > > > implicit zeroes as well, not just initial zeroes...
> > >
> > > Here's an updated patch that also considers intermediate implicit zeros.
> > >
> > > Bootstrap and regtest on x86_64-pc-linux-gnu in progress, does this look
> > > OK for
> > > trunk if successful?
> > >
> > > -- >8 --
> > >
> > > Subject: [PATCH] c++: mangling cNTTP object w/ implicit non-trailing zeros
> > > [PR121231]
> > >
> > > Here the result of A::make(0, 0, 1), (0, 1, 0) and (1, 0, 0) are each
> > > represented as a single-element CONSTRUCTOR with CONSTRUCTOR_NO_CLEARING
> > > cleared, and we end up mangling them all as A{1}, i.e. eliding both the
> > > implicit initial and trailing zeros. Mangling them all the same seems
> > > clearly wrong since they're logically different values.
> > >
> > > It turns out we also omit intermediate zeros, e.g. A::make(1, 0, 1) is
> > > mangled as A{1, 1} same as A::make(1, 1, 0).
> > >
> > > It seems we can't omit both trailing and non-trailing implicit zeros
> > > without introducing mangling ambiguities. This patch makes us include
> > > non-trailing zeros in these manglings, while continuing to omit
> > > trailing zeros, which matches e.g. Clang.
> > >
> > > PR c++/121231
> > >
> > > gcc/ChangeLog:
> > >
> > > * common.opt: Document additional ABI version 21 change.
> > > * doc/invoke.texi: Likewise.
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * mangle.cc (write_expression): Write out implicit non-trailing
> > > zeroes of a CONSTRUCTOR when the ABI version is at least 21.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/abi/mangle82.C: New test.
> > > ---
> > > gcc/common.opt | 2 +
> > > gcc/cp/mangle.cc | 49 ++++++++++++++++
> > > gcc/doc/invoke.texi | 5 +-
> > > gcc/testsuite/g++.dg/abi/mangle82.C | 86 +++++++++++++++++++++++++++++
> > > 4 files changed, 140 insertions(+), 2 deletions(-)
> > > create mode 100644 gcc/testsuite/g++.dg/abi/mangle82.C
> > >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index 70659fabebd5..bf38f60d194b 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -1067,6 +1067,8 @@ Driver Undocumented
> > > ;
> > > ; 21: Fix noexcept lambda capture pruning.
> > > ; Fix C++20 layout of base with all explicitly defaulted
> > > constructors.
> > > +; Fix mangling of class and array objects with implicitly
> > > +; zero-initialized non-trailing subojects.
> > > ; Default in G++ 16.
> > > ;
> > > ; Additional positive integers will be assigned as new versions of
> > > diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> > > index 13d5dedebd29..296e0f34552e 100644
> > > --- a/gcc/cp/mangle.cc
> > > +++ b/gcc/cp/mangle.cc
> > > @@ -3745,9 +3745,55 @@ write_expression (tree expr)
> > > || !zero_init_expr_p (ce->value))
> > > last_nonzero = i;
> > > + tree prev_field = NULL_TREE;
> > > + if (TREE_CODE (TREE_TYPE (expr)) == RECORD_TYPE)
> > > + prev_field = first_field (etype);
> > > + else if (TREE_CODE (TREE_TYPE (expr)) == ARRAY_TYPE)
> > > + prev_field = size_int (0);
> >
> > On second thought I think the logic is a little clearer if prev_field is
> > NULL_TREE on the first iteration, and we adjust the i == 0 logic
> > accordingly. Like so (passes initial testisg, full testing in progress):
> >
> > -- >8 --
> >
> > Here the result of A::make(0, 0, 1), (0, 1, 0) and (1, 0, 0) are each
> > represented as a single-element CONSTRUCTOR with CONSTRUCTOR_NO_CLEARING
> > cleared, and we end up mangling them all as A{1}, i.e. eliding both the
> > implicit initial and trailing zeros. Mangling them all the same seems
> > clearly wrong since they're logically different values.
> >
> > It turns out we also omit intermediate zeros, e.g. A::make(1, 0, 1) is
> > mangled as A{1, 1}, the same as A::make(1, 1, 0).
> >
> > It seems we can't omit both trailing and non-trailing implicit zeros
> > without introducing mangling ambiguities. This patch makes us include
> > non-trailing zeros in these manglings, while continuing to omit
> > trailing zeros, which matches e.g. Clang.
> >
> > PR c++/121231
> >
> > gcc/ChangeLog:
> >
> > * common.opt: Document additional ABI version 21 change.
> > * doc/invoke.texi: Likewise.
> >
> > gcc/cp/ChangeLog:
> >
> > * mangle.cc (write_expression): Write out implicit non-trailing
> > zeroes of a CONSTRUCTOR when the ABI version is at least 21.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/abi/mangle82.C: New test.
> > ---
> > gcc/common.opt | 2 +
> > gcc/cp/mangle.cc | 50 +++++++++++++++++
> > gcc/doc/invoke.texi | 5 +-
> > gcc/testsuite/g++.dg/abi/mangle82.C | 86 +++++++++++++++++++++++++++++
> > 4 files changed, 141 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/abi/mangle82.C
> >
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index 70659fabebd5..bf38f60d194b 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -1067,6 +1067,8 @@ Driver Undocumented
> > ;
> > ; 21: Fix noexcept lambda capture pruning.
> > ; Fix C++20 layout of base with all explicitly defaulted constructors.
> > +; Fix mangling of class and array objects with implicitly
> > +; zero-initialized non-trailing subojects.
> > ; Default in G++ 16.
> > ;
> > ; Additional positive integers will be assigned as new versions of
> > diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
> > index 13d5dedebd29..23b21c0a5b7a 100644
> > --- a/gcc/cp/mangle.cc
> > +++ b/gcc/cp/mangle.cc
> > @@ -3745,11 +3745,58 @@ write_expression (tree expr)
> > || !zero_init_expr_p (ce->value))
> > last_nonzero = i;
> > + tree prev_field = NULL_TREE;
> > if (undigested || last_nonzero != UINT_MAX)
> > for (HOST_WIDE_INT i = 0; vec_safe_iterate (elts, i, &ce);
> > ++i)
> > {
> > if (i > last_nonzero)
> > break;
> > + if (!undigested && !CONSTRUCTOR_NO_CLEARING (expr)
> > + && (TREE_CODE (etype) == RECORD_TYPE
> > + || TREE_CODE (etype) == ARRAY_TYPE))
> > + {
> > + /* Write out any implicit non-trailing zeros
> > + (which we neglected to do before v21). */
> > + if (TREE_CODE (etype) == RECORD_TYPE)
> > + {
> > + tree field;
> > + if (i == 0)
> > + field = first_field (etype);
> > + else
> > + field = DECL_CHAIN (prev_field);
> > + for (; field; field = DECL_CHAIN (field))
> > + {
> > + field = next_subobject_field (field);
> > + if (!field || field == ce->index)
> > + break;
> > + if (abi_check (21))
> > + write_expression (build_zero_cst
> > + (TREE_TYPE (field)));
>
> This seems to assume that field will have scalar type, surely that isn't
> necessarily true?
build_zero_cst also handles aggregate types by returning an empty
CONSTRUCTOR, which we in turn mangle as T{}.
>
> > + }
> > + }
> > + else if (TREE_CODE (etype) == ARRAY_TYPE)
> > + {
> > + unsigned HOST_WIDE_INT j;
> > + if (i == 0)
> > + j = 0;
> > + else
> > + j = 1 + tree_to_uhwi (prev_field);
> > + unsigned HOST_WIDE_INT k;
> > + if (TREE_CODE (ce->index) == RANGE_EXPR)
> > + k = tree_to_uhwi (TREE_OPERAND (ce->index, 0));
> > + else
> > + k = tree_to_uhwi (ce->index);
> > + tree zero = NULL_TREE;
> > + for (; j < k; ++j)
> > + if (abi_check (21))
> > + {
> > + if (!zero)
> > + zero = build_zero_cst (TREE_TYPE (etype));
> > + write_expression (zero);
> > + }
> > + }
> > + }
> > +
> > if (!undigested && TREE_CODE (etype) == UNION_TYPE)
> > {
> > /* Express the active member as a designator. */
> > @@ -3794,6 +3841,9 @@ write_expression (tree expr)
> > else
> > for (unsigned j = 0; j < reps; ++j)
> > write_expression (ce->value);
> > + prev_field = ce->index;
> > + if (prev_field && TREE_CODE (prev_field) == RANGE_EXPR)
> > + prev_field = TREE_OPERAND (prev_field, 1);
> > }
> > }
> > else
> > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > index e442a9cb73e4..61fcf5deebb8 100644
> > --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -3016,8 +3016,9 @@ Version 20, which first appeared in G++ 15, fixes
> > manglings of lambdas
> > in static data member initializers.
> > Version 21, which first appeared in G++ 16, fixes unnecessary captures
> > -in noexcept lambdas (c++/119764) and layout of a base class
> > -with all explicitly defaulted constructors (c++/120012).
> > +in noexcept lambdas (c++/119764), layout of a base class with all
> > explicitly
> > +defaulted constructors (c++/120012), and mangling of class and array
> > +objects with implicitly zero-initialized non-trailing subobjects
> > (c++/121231).
> > See also @option{-Wabi}.
> > diff --git a/gcc/testsuite/g++.dg/abi/mangle82.C
> > b/gcc/testsuite/g++.dg/abi/mangle82.C
> > new file mode 100644
> > index 000000000000..b93db9e77ddb
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/abi/mangle82.C
> > @@ -0,0 +1,86 @@
> > +// Test mangling of C++20 class NTTP objects with implicitly zeroed
> > +// non-trailing subojects.
> > +// PR c++/121231
> > +// { dg-do compile { target c++20 } }
> > +
> > +struct A {
> > + int x, y, z;
> > +
> > + static constexpr A make(int x, int y, int z) {
> > + A a{};
> > + if (x != 0)
> > + a.x = x;
> > + if (y != 0)
> > + a.y = y;
> > + if (z != 0)
> > + a.z = z;
> > + return a;
> > + }
> > +
> > +};
> > +
> > +struct B : A {
> > + int w;
> > +
> > + static constexpr B make(int x, int y, int z, int w) {
> > + B b{};
> > + if (x != 0 || y != 0 || z != 0)
> > + static_cast<A&>(b) = A::make(x, y, z);
> > + if (w != 0)
> > + b.w = w;
> > + return b;
> > + }
> > +};
> > +
> > +struct C {
> > + int xyz[3];
> > +
> > + static constexpr C make(int x, int y, int z) {
> > + C c{};
> > + if (x != 0)
> > + c.xyz[0] = x;
> > + if (y != 0)
> > + c.xyz[1] = y;
> > + if (z != 0)
> > + c.xyz[2] = z;
> > + return c;
> > + }
> > +};
> > +
> > +template<int N, A a> void f();
> > +template<int N, B b> void g();
> > +template<int N, C c> void h();
> > +
> > +int main() {
> > + f<0, A::make(0, 0, 1)>(); // void f<0, A{0, 0, 1}>()
> > + f<1, A::make(0, 1, 0)>(); // void f<1, A{0, 1}>()
> > + f<2, A::make(0, 0, 0)>(); // void f<2, A{}>()
> > + f<3, A::make(1, 0, 1)>(); // void f<3, A{1, 0, 1}>()
> > +
> > + g<0, B::make(0, 0, 0, 1)>(); // void g<0, B{A{}, 1}>()
> > + g<1, B::make(0, 0, 1, 0)>(); // void g<1, B{A{0, 0, 1}}>()
> > + g<2, B::make(0, 1, 0, 0)>(); // void g<2, B{A{0, 1}}>()
> > + g<3, B::make(0, 0, 0, 0)>(); // void g<3, B{}>()
> > + g<4, B::make(1, 0, 1, 0)>(); // void g<4, B{A{1, 0, 1}}>()
> > +
> > + h<0, C::make(0, 0, 1)>(); // void h<0, C{int [3]{0, 0, 1}}>()
> > + h<1, C::make(0, 1, 0)>(); // void h<1, C{int [3]{0, 1}}>()
> > + h<2, C::make(0, 0, 0)>(); // void h<2, C{}>()
> > + h<3, C::make(1, 0, 1)>(); // void h<3, C{int [3]{1, 0, 1}}>()
> > +}
> > +
> > +// { dg-final { scan-assembler "_Z1fILi0EXtl1ALi0ELi0ELi1EEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1fILi1EXtl1ALi0ELi1EEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1fILi2EXtl1AEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1fILi3EXtl1ALi1ELi0ELi1EEEEvv" } }
> > +
> > +// { dg-final { scan-assembler "_Z1gILi0EXtl1Btl1AELi1EEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1gILi1EXtl1Btl1ALi0ELi0ELi1EEEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1gILi2EXtl1Btl1ALi0ELi1EEEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1gILi3EXtl1BEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1gILi4EXtl1Btl1ALi1ELi0ELi1EEEEEvv" } }
> > +
> > +// { dg-final { scan-assembler "_Z1hILi0EXtl1CtlA3_iLi0ELi0ELi1EEEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1hILi1EXtl1CtlA3_iLi0ELi1EEEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1hILi2EXtl1CEEEvv" } }
> > +// { dg-final { scan-assembler "_Z1hILi3EXtl1CtlA3_iLi1ELi0ELi1EEEEEvv" } }
>
>
