auto operator=(segmented_vector&& other) noexcept -> segmented_vector& checks for allocator equality and if allocators are not equal the array is basically rebuilt (with elements moved individually). This is from the current code base:
auto operator=(segmented_vector&& other) noexcept -> segmented_vector& {
clear();
dealloc();
if (other.get_allocator() == get_allocator()) {
m_blocks = std::move(other.m_blocks);
m_size = std::exchange(other.m_size, {});
} else {
// make sure to construct with other's allocator!
m_blocks = std::vector<pointer, vec_alloc>(vec_alloc(other.get_allocator()));
append_everything_from(std::move(other));
}
return *this;
}
Here append_everything_from(std::move(other)); potentially allocates memory and any OOM situation immediately crashes straight into a noexcept boundary. There is also some twisted logic going on. After
// make sure to construct with other's allocator!
m_blocks = std::vector<pointer, vec_alloc>(vec_alloc(other.get_allocator()));
the condition other.get_allocator() == get_allocator() would always be true, right? Therefore, after
the contaner will always end up with other's allocator (regardless of whether alloc==other.get_allocator()). There is still a silly edge case in the operator= method I guess: While std::vector<pointer, vec_alloc>(vec_alloc(other.get_allocator())) actually has an allocator that compares equal to other.get_allocator(), it is unclear if this allocator will be propagated through the assignment which depends on pocma, so you can still end up with m_blocks.get_allocator()!=other.m_blocks.get_allocator() after m_blocks = std::vector<pointer, vec_alloc>(vec_alloc(other.get_allocator()));. Propose fix:
auto operator=(segmented_vector&& other) noexcept -> segmented_vector& {
static_assert(vec_alloc has pocma==true);
clear();
dealloc();
m_blocks = std::move(other.m_blocks); // without pocma m_block could end up with an incompatible allocator
m_size = std::exchange(other.m_size, {});
return *this;
}
Here are some other issues regarding allocators that should be rather easy to fix:
auto operator=(segmented_vector&& other) noexcept -> segmented_vector&
checks for allocator equality and if allocators are not equal the array is basically rebuilt (with elements moved individually). This is from the current code base:Here
append_everything_from(std::move(other));
potentially allocates memory and any OOM situation immediately crashes straight into a noexcept boundary. There is also some twisted logic going on. Afterthe condition
other.get_allocator() == get_allocator()
would always be true, right? Therefore, afterthe contaner will always end up with
other
's allocator (regardless of whetheralloc==other.get_allocator()
). There is still a silly edge case in theoperator=
method I guess: Whilestd::vector<pointer, vec_alloc>(vec_alloc(other.get_allocator()))
actually has an allocator that compares equal toother.get_allocator()
, it is unclear if this allocator will be propagated through the assignment which depends onpocma
, so you can still end up withm_blocks.get_allocator()!=other.m_blocks.get_allocator()
afterm_blocks = std::vector<pointer, vec_alloc>(vec_alloc(other.get_allocator()));
. Propose fix:Here are some other issues regarding allocators that should be rather easy to fix:
Allocator is not propagated as it should be.
Doesn't propagate the allocator of other:
This looks off - isn't this a use before init of
get_allocator()
?When it comes to the excellent (cough) allocator requirements, maybe an honest
would go a long way.