Fix vector insert of a value that refers to an element of the vector (#1630)

* Fix vector insert of a value that refers to an element of the vector

insert(position, value) and insert(position, n, value) shifted the
elements before reading value, so a value taken from the vector at or
after the insert position read the shifted element instead.

Fixes #1629

* Fix vector rvalue insert and emplace of a value that refers to an element of the vector

insert(position, T&&) moved from value after shifting the elements, so a
value referring to an element at or after the insert position read the
shifted element. It now uses the same address adjustment as the lvalue
insert.

emplace(position, args...) destroyed the element at the insert position
before constructing the new one, so arguments referring to elements of
the vector could read a shifted or destroyed element. The new value is
now constructed before the elements are shifted, for both the variadic
and the C++03 overloads.

* Keep the emplace comment after the diagnostic include for clang-format

clang-format indented the comment to align it with the preprocessor line
that followed it, so the formatting check failed on vector.h.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ce8Hr56xHurqiV5wEkVmxj

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: John Wellbelove <jwellbelove@users.noreply.github.com>
This commit is contained in:
Cristian 2026-10-31 11:36:18 +02:00 committed by GitHub
parent 5fe4a6cbd0
commit b2c437c13e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 261 additions and 42 deletions

View File

@ -611,9 +611,18 @@ namespace etl
}
else
{
// value may refer to an element that is about to be shifted.
const_pointer p_value = etl::addressof(value);
create_back(back());
etl::move_backward(position_, p_end - 2, p_end - 1);
*position_ = value;
if ((p_value >= position_) && (p_value < p_end))
{
++p_value;
}
*position_ = *p_value;
}
return position_;
@ -640,9 +649,18 @@ namespace etl
}
else
{
// value may refer to an element that is about to be shifted.
pointer p_value = etl::addressof(value);
create_back(etl::move(back()));
etl::move_backward(position_, p_end - 2, p_end - 1);
*position_ = etl::move(value);
if ((p_value >= position_) && (p_value < p_end))
{
++p_value;
}
*position_ = etl::move(*p_value);
}
return position_;
@ -661,25 +679,26 @@ namespace etl
iterator position_ = to_iterator(position);
void* p;
if (position_ == end())
{
p = p_end++;
#include "private/diagnostic_sign_conversion_push.h"
::new (p_end) T(etl::forward<Args>(args)...);
#include "private/diagnostic_pop.h"
++p_end;
ETL_INCREMENT_DEBUG_COUNT;
}
else
{
p = etl::addressof(*position_);
create_back(back());
etl::move_backward(position_, p_end - 2, p_end - 1);
(*position_).~T();
}
#include "private/diagnostic_sign_conversion_push.h"
::new (p) T(etl::forward<Args>(args)...);
// args may refer to elements that are about to be shifted, so construct the value first.
T value(etl::forward<Args>(args)...);
#include "private/diagnostic_pop.h"
create_back(etl::move(back()));
etl::move_backward(position_, p_end - 2, p_end - 1);
*position_ = etl::move(value);
}
return position_;
}
#else
@ -691,23 +710,22 @@ namespace etl
iterator position_ = to_iterator(position);
void* p;
if (position_ == end())
{
p = p_end++;
::new (p_end) T(value1);
++p_end;
ETL_INCREMENT_DEBUG_COUNT;
}
else
{
p = etl::addressof(*position_);
// The values may refer to elements that are about to be shifted, so construct the value first.
T value(value1);
create_back(back());
etl::move_backward(position_, p_end - 2, p_end - 1);
(*position_).~T();
*position_ = value;
}
::new (p) T(value1);
return position_;
}
@ -719,23 +737,22 @@ namespace etl
iterator position_ = to_iterator(position);
void* p;
if (position_ == end())
{
p = p_end++;
::new (p_end) T(value1, value2);
++p_end;
ETL_INCREMENT_DEBUG_COUNT;
}
else
{
p = etl::addressof(*position_);
// The values may refer to elements that are about to be shifted, so construct the value first.
T value(value1, value2);
create_back(back());
etl::move_backward(position_, p_end - 2, p_end - 1);
(*position_).~T();
*position_ = value;
}
::new (p) T(value1, value2);
return position_;
}
@ -747,23 +764,22 @@ namespace etl
iterator position_ = to_iterator(position);
void* p;
if (position_ == end())
{
p = p_end++;
::new (p_end) T(value1, value2, value3);
++p_end;
ETL_INCREMENT_DEBUG_COUNT;
}
else
{
p = etl::addressof(*position_);
// The values may refer to elements that are about to be shifted, so construct the value first.
T value(value1, value2, value3);
create_back(back());
etl::move_backward(position_, p_end - 2, p_end - 1);
(*position_).~T();
*position_ = value;
}
::new (p) T(value1, value2, value3);
return position_;
}
@ -775,23 +791,22 @@ namespace etl
iterator position_ = to_iterator(position);
void* p;
if (position_ == end())
{
p = p_end++;
::new (p_end) T(value1, value2, value3, value4);
++p_end;
ETL_INCREMENT_DEBUG_COUNT;
}
else
{
p = etl::addressof(*position_);
// The values may refer to elements that are about to be shifted, so construct the value first.
T value(value1, value2, value3, value4);
create_back(back());
etl::move_backward(position_, p_end - 2, p_end - 1);
(*position_).~T();
*position_ = value;
}
::new (p) T(value1, value2, value3, value4);
return position_;
}
#endif
@ -836,6 +851,14 @@ namespace etl
size_t copy_new_n = construct_old_n;
size_t construct_new_n = insert_n - copy_new_n;
// value may refer to an element that is about to be shifted.
const_pointer p_value = etl::addressof(value);
if ((p_value >= position_) && (p_value < p_end))
{
p_value += n;
}
// Construct old.
etl::uninitialized_move(p_end - construct_old_n, p_end, p_construct_old);
ETL_ADD_DEBUG_COUNT(construct_old_n);
@ -844,11 +867,11 @@ namespace etl
etl::move_backward(p_buffer + insert_begin, p_buffer + insert_begin + copy_old_n, p_buffer + insert_end + copy_old_n);
// Construct new.
etl::uninitialized_fill_n(p_end, construct_new_n, value);
etl::uninitialized_fill_n(p_end, construct_new_n, *p_value);
ETL_ADD_DEBUG_COUNT(construct_new_n);
// Copy new.
etl::fill_n(p_buffer + insert_begin, copy_new_n, value);
etl::fill_n(p_buffer + insert_begin, copy_new_n, *p_value);
p_end += n;
}

View File

@ -777,6 +777,62 @@ namespace
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_insert_position_value_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
Compare_Data compare_data;
Data data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
data.insert(data.cbegin() + offset, data[source]);
compare_data.insert(compare_data.cbegin() + static_cast<ptrdiff_t>(offset), compare_data[source]);
CHECK_EQUAL(compare_data.size(), data.size());
bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin());
CHECK(is_equal);
}
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_insert_position_rvalue_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
Compare_Data compare_data;
Data data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
data.insert(data.cbegin() + offset, std::move(data[source]));
int value = compare_data[source];
compare_data.insert(compare_data.cbegin() + static_cast<ptrdiff_t>(offset), value);
CHECK_EQUAL(compare_data.size(), data.size());
bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin());
CHECK(is_equal);
}
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_insert_position_value_outofbounds)
{
@ -814,6 +870,35 @@ namespace
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_emplace_position_value_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
Compare_Data compare_data;
Data data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
data.emplace(data.cbegin() + offset, data[source]);
int value = compare_data[source];
compare_data.insert(compare_data.cbegin() + static_cast<ptrdiff_t>(offset), value);
CHECK_EQUAL(compare_data.size(), data.size());
bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin());
CHECK(is_equal);
}
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_emplace_position_value_outofbounds)
{
@ -971,6 +1056,36 @@ namespace
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_insert_position_n_value_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t insert_size = 1; insert_size <= 4; ++insert_size)
{
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
Compare_Data compare_data;
Data data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
data.insert(data.cbegin() + offset, insert_size, data[source]);
compare_data.insert(compare_data.cbegin() + static_cast<ptrdiff_t>(offset), insert_size, compare_data[source]);
CHECK_EQUAL(compare_data.size(), data.size());
bool is_equal = std::equal(data.cbegin(), data.cend(), compare_data.cbegin());
CHECK(is_equal);
}
}
}
}
//*************************************************************************
#include "etl/private/diagnostic_uninitialized_push.h"
TEST_FIXTURE(SetupFixture, test_insert_position_n_value_outofbounds)

View File

@ -995,6 +995,87 @@ namespace
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_emplace_position_value_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
CompareDataNDC compare_data;
DataNDC data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
data.emplace(data.begin() + offset, data[source]);
NDC value(compare_data[source]);
compare_data.insert(compare_data.begin() + static_cast<ptrdiff_t>(offset), value);
CHECK_EQUAL(compare_data.size(), data.size());
bool is_equal = std::equal(data.begin(), data.end(), compare_data.begin());
CHECK(is_equal);
}
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_emplace_position_member_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
CompareDataNDC compare_data;
DataNDC data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
compare_data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
data.emplace(data.begin() + offset, data[source].value, data[source].index);
NDC value(compare_data[source].value, compare_data[source].index);
compare_data.insert(compare_data.begin() + static_cast<ptrdiff_t>(offset), value);
CHECK_EQUAL(compare_data.size(), data.size());
bool is_equal = std::equal(data.begin(), data.end(), compare_data.begin());
CHECK(is_equal);
}
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_insert_position_rvalue_from_self)
{
const size_t INITIAL_SIZE = 5;
for (size_t offset = 0; offset <= INITIAL_SIZE; ++offset)
{
for (size_t source = 0; source < INITIAL_SIZE; ++source)
{
DataNDC data;
data.assign(initial_data.begin(), initial_data.begin() + INITIAL_SIZE);
const NDC expected(data[source]);
data.insert(data.begin() + offset, std::move(data[source]));
CHECK_EQUAL(INITIAL_SIZE + 1, data.size());
CHECK(expected == data[offset]);
}
}
}
//*************************************************************************
TEST_FIXTURE(SetupFixture, test_emplace_position_value_outofbounds)
{