On Tue, Sep 8, 2026 at 11:15 PM feedable <[email protected]> wrote:
>
>
> On 08/09/2026 11:34, Richard Biener wrote:
> > On Sat, Sep 5, 2026 at 7:39 PM feedable <[email protected]> wrote:
> >>
> >> On 25/08/2026 13:48, Richard Biener wrote:
> >>> On Tue, Aug 25, 2026 at 1:26 AM feedable via Sourceware Forge
> >>> <[email protected]> wrote:
> >>>> From: feedable <[email protected]>
> >>>>
> >>>> Currently, auto_vec defaults its copy&move operations. This is bad,
> >>>> since a user that tries to copy or move an auto_vec will unknowingly
> >>>> set himself up for trouble, since the pointers from the base and members
> >>>> will be copied verbatim, leading to double free situtaions and the like.
> >>> For auto_vec<T, N> with N != 0 we do not want these, instead they should
> >>> be deleted. As seen in your implementation the complexity involved with
> >>> having to deal with the stack backing for small number of elements makes
> >>> this very inefficient. If copy/move is needed people should use
> >>> auto_vec<T, 0>
> >>> instead.
> >> As stated in the discussion for v1, a move constructor is required for a
> >> value to be returned. Values of type auto_vec<T, N> with N != 0 are
> >> returned in the codebase already (yes, this is currently UB if the
> >> constructor is not elided).
> > IMO those uses should be fixed.
> I'm sorry but I still fail to understand the purpose of forbidding those
> operations, given that an implementation of these operations is present
> in this patch.
>
> It is fairly clear to me that forbidding these operations will incur
> costs on the codebase, such as denying storage of `auto_vec`s in
> containers (which is being used in the following patch), and denying the
> ability to return them (which is used now in the code elsewhere).
>
> What is not clear to me is the benefits of such a decision.
>
> Regarding the efficiency of my implementation of those ops, claims about
> that can only be made in comparison, and the alternative here is to
> unconditionally allocate on the heap, which is worse than storing the
> data inline and eliding the move on return.
I'm not sure it is worse.
> Until the net-benefits are clear to me, I cannot in good faith move
> forward with this change.
The argument is that once you expose an inefficient API you'll
get lazy uses and inefficiencies that could have been easily avoided.
But I might be overly pessimistic here, not knowing how often the
copy/move construction/assignment ops for auto_vec would occur.
There is also the semantic difference with vec<> where
assignment does a shallow copy of the m_vec pointer and _not_
copy the data as with your non-move assignment/ctor. So can
we at least avoid that? vec<> has vec<>::copy for this and
truncate() + splice() for the assignment.
We should possibly delete vec<>::operator=() and adjust code
to use const vec<>& argument passing (it's one more indirection
sadly). Adding move assignment might be useful, but I'd like
to explicitly "see" it, like if we can force v1 = std::move (v2); or so?
Richard.
> >>>> Introduce proper constructor and operator overloads, that perform proper
> >>>> transfer of memory controlled by auto_vec.
> >>>>
> >>>> gcc/ChangeLog:
> >>>>
> >>>> * vec.cc (struct count_cdtors): Test utility type.
> >>>> (test_auto_vec_copy_move_construct): New test.
> >>>> (test_auto_vec_move_assign): Likewise.
> >>>> (test_auto_vec_copy_assign): Likewise.
> >>>> (vec_cc_tests): Likewise.
> >>>> * vec.h (vec<T, A, vl_embed>::quick_emplace): New function.
> >>>> (vec<T, va_heap, vl_ptr>::quick_emplace): Likewise.
> >>>> (vec<T, va_heap, vl_ptr>::safe_emplace): Likewise.
> >>>> ---
> >>>> gcc/vec.cc | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>> gcc/vec.h | 129 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 272 insertions(+)
> >>>>
> >>>> diff --git a/gcc/vec.cc b/gcc/vec.cc
> >>>> index 8313adde2b22b..3d7fc387ba428 100644
> >>>> --- a/gcc/vec.cc
> >>>> +++ b/gcc/vec.cc
> >>>> @@ -640,6 +640,146 @@ test_auto_delete_vec ()
> >>>> ASSERT_EQ (dtor_count, 2);
> >>>> }
> >>>>
> >>>> +struct count_cdtors{
> >>>> + static int copy_cnt, move_cnt, ctor_cnt, dtor_cnt;
> >>>> + static void reset ()
> >>>> + {
> >>>> + copy_cnt = 0;
> >>>> + move_cnt = 0;
> >>>> + ctor_cnt = 0;
> >>>> + dtor_cnt = 0;
> >>>> + }
> >>>> + static void validate (int copy, int move, int ctor, int dtor)
> >>>> + {
> >>>> + ASSERT_EQ (copy_cnt, copy);
> >>>> + ASSERT_EQ (move_cnt, move);
> >>>> + ASSERT_EQ (ctor_cnt, ctor);
> >>>> + ASSERT_EQ (dtor_cnt, dtor);
> >>>> + reset ();
> >>>> + }
> >>>> + count_cdtors () { ++ctor_cnt; }
> >>>> + ~count_cdtors () { ++dtor_cnt; }
> >>>> + count_cdtors (count_cdtors &&) { ++move_cnt; }
> >>>> + count_cdtors (const count_cdtors &) { ++copy_cnt; }
> >>>> + count_cdtors &operator= (count_cdtors &&) { return ++move_cnt, *this;
> >>>> }
> >>>> + count_cdtors &operator= (const count_cdtors &) { return ++copy_cnt,
> >>>> *this; }
> >>>> +};
> >>>> +int count_cdtors::copy_cnt = 0,
> >>>> + count_cdtors::move_cnt = 0,
> >>>> + count_cdtors::ctor_cnt = 0,
> >>>> + count_cdtors::dtor_cnt = 0;
> >>>> +
> >>>> +static void
> >>>> +test_auto_vec_copy_move_construct ()
> >>>> +{
> >>>> + count_cdtors::reset ();
> >>>> + {
> >>>> + count_cdtors::validate (0, 0, 0, 0);
> >>>> + auto_vec<count_cdtors, 1> test (2);
> >>>> + count_cdtors::validate (0, 0, 0, 0);
> >>>> + test.quick_grow_cleared (2);
> >>>> + count_cdtors::validate (0, 0, 2, 0);
> >>>> + auto test2 = test;
> >>>> + count_cdtors::validate (2, 0, 0, 0);
> >>>> + auto test3 = std::move (test);
> >>>> + count_cdtors::validate (0, 0, 0, 0);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 4);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 2> test (2);
> >>>> + count_cdtors::validate (0, 0, 0, 0);
> >>>> + test.quick_grow_cleared (2);
> >>>> + count_cdtors::validate (0, 0, 2, 0);
> >>>> + auto test2 = test;
> >>>> + count_cdtors::validate (2, 0, 0, 0);
> >>>> + auto test3 = std::move (test);
> >>>> + count_cdtors::validate (0, 2, 0, 0);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 6);
> >>>> +}
> >>>> +
> >>>> +static void
> >>>> +test_auto_vec_move_assign ()
> >>>> +{
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (1), to (1);
> >>>> + from.quick_grow_cleared (1);
> >>>> + to.quick_grow_cleared (1);
> >>>> + count_cdtors::validate (0, 0, 2, 0);
> >>>> + to = std::move (from);
> >>>> + count_cdtors::validate (0, 1, 0, 0);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (1), to (2);
> >>>> + from.quick_grow_cleared (1);
> >>>> + to.quick_grow_cleared (2);
> >>>> + count_cdtors::validate (0, 0, 3, 0);
> >>>> + to = std::move (from);
> >>>> + count_cdtors::validate (0, 1, 0, 2);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (2), to (1);
> >>>> + from.quick_grow_cleared (2);
> >>>> + to.quick_grow_cleared (1);
> >>>> + count_cdtors::validate (0, 0, 3, 0);
> >>>> + to = std::move (from);
> >>>> + count_cdtors::validate (0, 0, 0, 1);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (2), to (2);
> >>>> + from.quick_grow_cleared (2);
> >>>> + to.quick_grow_cleared (2);
> >>>> + count_cdtors::validate (0, 0, 4, 0);
> >>>> + to = std::move (from);
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> +}
> >>>> +
> >>>> +static void
> >>>> +test_auto_vec_copy_assign ()
> >>>> +{
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (1), to (1);
> >>>> + from.quick_grow_cleared (1);
> >>>> + to.quick_grow_cleared (1);
> >>>> + count_cdtors::validate (0, 0, 2, 0);
> >>>> + to = from;
> >>>> + count_cdtors::validate (1, 0, 0, 0);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (1), to (2);
> >>>> + from.quick_grow_cleared (1);
> >>>> + to.quick_grow_cleared (2);
> >>>> + count_cdtors::validate (0, 0, 3, 0);
> >>>> + to = from;
> >>>> + count_cdtors::validate (1, 0, 0, 1);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 2);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (2), to (1);
> >>>> + from.quick_grow_cleared (2);
> >>>> + to.quick_grow_cleared (1);
> >>>> + count_cdtors::validate (0, 0, 3, 0);
> >>>> + to = from;
> >>>> + count_cdtors::validate (2, 0, 0, 1);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 4);
> >>>> + {
> >>>> + auto_vec<count_cdtors, 1> from (2), to (2);
> >>>> + from.quick_grow_cleared (2);
> >>>> + to.quick_grow_cleared (2);
> >>>> + count_cdtors::validate (0, 0, 4, 0);
> >>>> + to = from;
> >>>> + count_cdtors::validate (2, 0, 0, 0);
> >>>> + }
> >>>> + count_cdtors::validate (0, 0, 0, 4);
> >>>> +}
> >>>> +
> >>>> /* Verify accesses to vector elements are done indirectly. */
> >>>>
> >>>> static void
> >>>> @@ -828,6 +968,9 @@ vec_cc_tests ()
> >>>> test_qsort ();
> >>>> test_reverse ();
> >>>> test_auto_delete_vec ();
> >>>> + test_auto_vec_copy_move_construct ();
> >>>> + test_auto_vec_copy_assign ();
> >>>> + test_auto_vec_move_assign ();
> >>>> test_auto_alias ();
> >>>> test_string_slice_initializers ();
> >>>> test_string_slice_tokenize ();
> >>>> diff --git a/gcc/vec.h b/gcc/vec.h
> >>>> index 9d25678029832..fcab26d5c418d 100644
> >>>> --- a/gcc/vec.h
> >>>> +++ b/gcc/vec.h
> >>>> @@ -620,6 +620,8 @@ public:
> >>>> void splice (const vec &);
> >>>> void splice (const vec *src);
> >>>> T *quick_push (const T &);
> >>>> + template<typename ...Ts>
> >>>> + T *quick_emplace (Ts &&...);
> >>>> using pop_ret_type
> >>>> = typename std::conditional <std::is_trivially_destructible
> >>>> <T>::value,
> >>>> T &, void>::type;
> >>>> @@ -1053,6 +1055,16 @@ vec<T, A, vl_embed>::quick_push (const T &obj)
> >>>> return slot;
> >>>> }
> >>>>
> >>>> +template<typename T, typename A>
> >>>> +template<typename ...Ts>
> >>>> +inline T *
> >>>> +vec<T, A, vl_embed>::quick_emplace (Ts &&...obj)
> >>>> +{
> >>>> + gcc_checking_assert (space (1));
> >>>> + T *slot = &address ()[m_vecpfx.m_num++];
> >>>> + ::new (static_cast<void*>(slot)) T ((Ts &&)obj...);
> >>>> + return slot;
> >>>> +}
> >>>>
> >>>> /* Pop and return a reference to the last element off the end of the
> >>>> vector. If T has non-trivial destructor, this method just pops
> >>>> @@ -1623,7 +1635,11 @@ public:
> >>>> void splice (const vec &);
> >>>> void safe_splice (const vec & CXX_MEM_STAT_INFO);
> >>>> T *quick_push (const T &);
> >>>> + template<typename ...Ts>
> >>>> + T *quick_emplace (Ts &&...);
> >>>> T *safe_push (const T &CXX_MEM_STAT_INFO);
> >>>> + template<typename ...Ts>
> >>>> + T *safe_emplace (Ts &&...);
> >>>> using pop_ret_type
> >>>> = typename std::conditional <std::is_trivially_destructible
> >>>> <T>::value,
> >>>> T &, void>::type;
> >>>> @@ -1691,6 +1707,91 @@ public:
> >>>> size_t off = (char *) &m_auto - (char *) this;
> >>>> this->m_vec = (vec<T, va_heap, vl_embed> *) ((char *) this + off);
> >>>> }
> >>>> + auto_vec (size_t s CXX_MEM_STAT_INFO, T fill): auto_vec (s)
> >>>> + {
> >>>> + for (size_t i = 0; i != s; ++i)
> >>>> + this->quick_push (fill);
> >>>> + }
> >>>> +
> >>>> + auto_vec (auto_vec &&r)
> >>>> + noexcept (std::is_nothrow_move_constructible<T>::value)
> >>>> + : auto_vec ()
> >>>> + {
> >>>> + if (!r.using_auto_storage ())
> >>>> + {
> >>>> + this->m_vec = std::exchange (r.m_vec, nullptr);
> >>>> + return;
> >>>> + }
> >>>> + for (auto &v: r)
> >>>> + this->quick_emplace (std::move (v));
> >>>> + }
> >>>> +
> >>>> + auto_vec &operator= (auto_vec &&r)
> >>>> + noexcept (std::is_nothrow_move_constructible<T>::value)
> >>>> + {
> >>>> + if (!r.using_auto_storage () && !this->using_auto_storage ())
> >>>> + {
> >>>> + std::swap (this->m_vec, r.m_vec);
> >>>> + r.release ();
> >>>> + }
> >>>> + else if (!r.using_auto_storage () && this->using_auto_storage ())
> >>>> + {
> >>>> + auto data = std::exchange (r.m_vec, nullptr);
> >>>> + std::exchange (this->m_vec, data)->truncate (0);
> >>>> + }
> >>>> + else if (r.using_auto_storage () && !this->using_auto_storage ())
> >>>> + {
> >>>> + /* To avoid losing temporaries, we have to fill the internal
> >>>> array
> >>>> + first, and only then destroy the residual elems. */
> >>>> + m_auto.embedded_init (N, 0, 1);
> >>>> + vec<T, va_heap> replacement;
> >>>> + size_t off = (char *) &m_auto - (char *) this;
> >>>> + replacement.m_vec = (vec<T, va_heap, vl_embed> *) ((char *) this
> >>>> + off);
> >>>> + for (auto &v: r)
> >>>> + replacement.quick_emplace (std::move (v));
> >>>> + std::exchange ((vec<T, va_heap> &)*this, replacement).release ();
> >>>> + }
> >>>> + else if (r.using_auto_storage () && this->using_auto_storage ())
> >>>> + {
> >>>> + /* The most complicated case: move-assign all the elements that
> >>>> are live
> >>>> + in both, rthen move-construct or destruct the rest. */
> >>>> + int a = this->length (), b = r.length ();
> >>>> + int i;
> >>>> + for (i = 0; i < a && i < b; ++i)
> >>>> + (*this)[i] = std::move (r[i]);
> >>>> + for (; i < b; ++i)
> >>>> + this->quick_emplace (std::move (r[i]));
> >>>> + this->truncate (b);
> >>>> + }
> >>>> + else gcc_unreachable ();
> >>>> + return *this;
> >>>> + }
> >>>> +
> >>>> + auto_vec (const auto_vec &r): auto_vec (r.length ())
> >>>> + {
> >>>> + for (auto &v: r)
> >>>> + this->quick_push (v);
> >>>> + }
> >>>> +
> >>>> + auto_vec &operator= (const auto_vec &r)
> >>>> + {
> >>>> + if (r.length () <= this->allocated ())
> >>>> + {
> >>>> + int a = this->length (), b = r.length ();
> >>>> + int i;
> >>>> + for (i = 0; i < a && i < b; ++i)
> >>>> + (*this)[i] = r[i];
> >>>> + for (; i < b; ++i)
> >>>> + this->quick_emplace (r[i]);
> >>>> + this->truncate (b);
> >>>> + }
> >>>> + else
> >>>> + {
> >>>> + auto_vec fresh = r;
> >>>> + *this = std::move (fresh);
> >>>> + }
> >>>> + return *this;
> >>>> + }
> >>>>
> >>>> ~auto_vec ()
> >>>> {
> >>>> @@ -2082,6 +2183,19 @@ vec<T, va_heap, vl_ptr>::quick_push (const T &obj)
> >>>> }
> >>>>
> >>>>
> >>>> +/* Construct a new element from constructor ARGS at the end of the
> >>>> vector.
> >>>> + There must be sufficient space in the vector. Return a pointer to
> >>>> the slot
> >>>> + where the element was inserted. */
> >>>> +
> >>>> +template<typename T>
> >>>> +template<typename ...Ts>
> >>>> +inline T *
> >>>> +vec<T, va_heap, vl_ptr>::quick_emplace (Ts &&...args)
> >>>> +{
> >>>> + return m_vec->quick_emplace ((Ts &&)args...);
> >>>> +}
> >>>> +
> >>>> +
> >>>> /* Push a new element OBJ onto the end of this vector. Reallocates
> >>>> the embedded vector, if needed. Return a pointer to the slot where
> >>>> OBJ was inserted. */
> >>>> @@ -2095,6 +2209,21 @@ vec<T, va_heap, vl_ptr>::safe_push (const T &obj
> >>>> MEM_STAT_DECL)
> >>>> }
> >>>>
> >>>>
> >>>> +/* Construct a new element from constructor ARGS at the end of the
> >>>> vector.
> >>>> + Reallocates the embedded vector, if needed. Return a pointer to the
> >>>> slot
> >>>> + where the element was inserted. */
> >>>> +
> >>>> +template<typename T>
> >>>> +template<typename ...Ts>
> >>>> +inline T *
> >>>> +vec<T, va_heap, vl_ptr>::safe_emplace (Ts &&...args)
> >>>> +{
> >>>> + reserve (1, false PASS_MEM_STAT);
> >>>> + return quick_emplace ((Ts &&)args...);
> >>>> +}
> >>>> +
> >>>> +
> >>>> +
> >>>> /* Pop and return a reference to the last element off the end of the
> >>>> vector. If T has non-trivial destructor, this method just pops
> >>>> last element and returns void. */
> >>>> --
> >>>> 2.54.0
> >>>>
