Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bugs in vector_impl<>::make_space_for_insert(). #29

Merged
merged 2 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions include/psi/vm/containers/crt_vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -407,10 +407,10 @@ class [[ nodiscard, clang::trivial_abi ]] crt_vector
std::swap( this->p_array_ , other.p_array_ );
std::swap( this->size_ , other.size_ );
std::swap( this->capacity_, other.capacity_ );
other.free();
other.clear();
return *this;
}
constexpr ~crt_vector() noexcept { free(); }
constexpr ~crt_vector() noexcept { base::clear(); }

[[ nodiscard, gnu::pure ]] size_type size () const noexcept { return size_; }
[[ nodiscard, gnu::pure ]] size_type capacity() const noexcept
Expand Down Expand Up @@ -485,6 +485,12 @@ private: friend base;
void storage_dec_size() noexcept { BOOST_ASSUME( size_ >= 1 ); --size_; }
void storage_inc_size() noexcept; // TODO

void storage_free() noexcept
{
al::deallocate( data(), capacity() );
mark_freed();
}

private:
[[ gnu::cold, gnu::noinline, clang::preserve_most ]]
void do_grow( size_type const target_size, size_type const cached_current_capacity )
Expand Down Expand Up @@ -514,13 +520,6 @@ private: friend base;
}
}

void free() noexcept
{
std::destroy_n( data(), size() );
al::deallocate( data(), capacity() );
mark_freed();
}

void mark_freed() noexcept { std::memset( this, 0, sizeof( *this ) ); }

private:
Expand Down
6 changes: 4 additions & 2 deletions include/psi/vm/containers/static_vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,10 @@ private: friend base;
BOOST_ASSUME( size_ >= target_size );
size_ = target_size;
}
void storage_dec_size() noexcept { BOOST_ASSUME( size_ >= 1 ); --size_; }
void storage_inc_size() noexcept; // TODO
constexpr void storage_dec_size() noexcept { BOOST_ASSUME( size_ >= 1 ); --size_; }
constexpr void storage_inc_size() noexcept; // TODO

constexpr void storage_free() noexcept { size_ = 0; }

private:
void fixed_copy( static_vector const & __restrict source ) noexcept
Expand Down
24 changes: 19 additions & 5 deletions include/psi/vm/containers/vector_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <cstring>
#include <iterator>
#include <memory>
#include <ranges>
#include <span>
#include <type_traits>
#include <std_fix/const_iterator.hpp>
Expand Down Expand Up @@ -756,7 +757,7 @@ class [[ nodiscard, clang::trivial_abi ]] vector_impl
void clear( this Impl & self ) noexcept
{
std::destroy( self.begin(), self.end() );
self.free();
self.storage_free();
}

void swap( this auto & self, auto & other ) noexcept { std::swap( self, other ); }
Expand Down Expand Up @@ -858,14 +859,27 @@ class [[ nodiscard, clang::trivial_abi ]] vector_impl
auto const elements_to_move{ static_cast<size_type>( current_size - position_index ) };
if constexpr ( is_trivially_moveable<value_type> )
{
std::uninitialized_move_n( &data[ position_index ], elements_to_move, &data[ position_index + n ] );
// does not use is_trivially_moveable/trivial_abi and is incorrect
// (i.e. an uninitialized_move_backwards is required)
//std::uninitialized_move_n( &data[ position_index ], elements_to_move, &data[ position_index + n ] );
std::memmove( &data[ position_index + n ], &data[ position_index ], elements_to_move * sizeof( *data ) );
}
else // future support for generic types
else
{
auto const elements_to_move_to_uninitialized_space{ n };
auto const elements_to_move_to_the_current_end { static_cast<size_type>( elements_to_move - elements_to_move_to_uninitialized_space ) };
std::uninitialized_move_n( &data[ current_size - elements_to_move_to_uninitialized_space ], elements_to_move_to_uninitialized_space, &data[ current_size ] );
std::move ( &data[ position_index ], &data[ position_index + elements_to_move_to_the_current_end ], &data[ position_index + n ] );
std::uninitialized_move
(
&data[ current_size - elements_to_move_to_uninitialized_space ],
&data[ current_size ],
&data[ current_size ]
);
std::move_backward
(
&data[ position_index ],
&data[ position_index + elements_to_move_to_the_current_end ],
&data[ position_index + elements_to_move_to_the_current_end + n ]
);
}
return self.make_iterator( &data[ position_index ] );
}
Expand Down
7 changes: 4 additions & 3 deletions include/psi/vm/containers/vm_vector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,13 +364,15 @@ private: friend vec_impl;
//! n; otherwise, capacity() is unchanged. In either case, size() is unchanged.
//!
//! <b>Throws</b>: If memory allocation allocation throws or T's copy/move constructor throws.
T * storage_shrink_to( sz_t const target_size ) noexcept { return static_cast<T *>( base::shrink_to( to_byte_sz( target_size ) ) ); }
T * storage_grow_to ( sz_t const target_size ) { return static_cast<T *>( base::grow_to ( to_byte_sz( target_size ) ) ); }
T * storage_shrink_to( sz_t const target_size ) noexcept { return static_cast<T *>( base::shrink_to( to_byte_sz( target_size ) ) ); }

void storage_shrink_size_to( sz_t const new_size ) noexcept { base::shrink_size_to( to_byte_sz( new_size ) ); }
void storage_dec_size() noexcept { storage_shrink_size_to( size() - 1 ); }
void storage_inc_size() noexcept; // TODO

void storage_free() noexcept { base::free(); }

protected:
PSI_WARNING_DISABLE_PUSH()
PSI_WARNING_GCC_OR_CLANG_DISABLE( -Wsign-conversion )
Expand Down Expand Up @@ -471,8 +473,7 @@ class [[ clang::trivial_abi ]] vm_vector
{
private:
using storage_t = typed_contiguous_storage<T, sz_t, headerless_param>;
//using impl = vector_impl<T, sz_t>;
using impl = storage_t;
using impl = storage_t;

public:
static constexpr auto headerless{ headerless_param };
Expand Down
32 changes: 20 additions & 12 deletions test/vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,33 +44,41 @@ TEST(vector_test, element_access) {


TEST(vector_test, modifiers) {
crt_vector<int> vec;
using test_str_t = std::conditional_t<is_trivially_moveable<std::string>, std::string, std::string_view>;
crt_vector<test_str_t> vec;

// Test push_back
vec.push_back(1);
vec.push_back(2);
vec.emplace_back("1");
vec.emplace_back("2");
EXPECT_EQ(vec.size(), 2);
EXPECT_EQ(vec[1], 2);
EXPECT_EQ(vec[1], "2");

// Test emplace_back
vec.emplace_back(3);
vec.emplace_back("3");
EXPECT_EQ(vec.size(), 3);
EXPECT_EQ(vec[2], 3);
EXPECT_EQ(vec[2], "3");

// Test pop_back
vec.pop_back();
EXPECT_EQ(vec.size(), 2);
EXPECT_EQ(vec.back(), 2);
EXPECT_EQ(vec.back(), "2");

// Test insert
vec.insert(vec.begin(), 0);
EXPECT_EQ(vec.front(), 0);
EXPECT_EQ(vec.size(), 3);
std::string_view const sbo_overflower{ "01234567898765432100123456789876543210" };
vec.emplace( vec.end () , sbo_overflower );
vec.emplace( vec.begin() , "0" );
vec.emplace( vec.nth( 3 ), "3" );
EXPECT_EQ( vec.front(), "0" );
EXPECT_EQ( vec[ 1 ] , "1" );
EXPECT_EQ( vec[ 2 ] , "2" );
EXPECT_EQ( vec[ 3 ] , "3" );
EXPECT_EQ( vec[ 4 ] , sbo_overflower );
EXPECT_EQ( vec.size(), 5 );

// Test erase
vec.erase(vec.begin());
EXPECT_EQ(vec.front(), 1);
EXPECT_EQ(vec.size(), 2);
EXPECT_EQ(vec.front(), "1");
EXPECT_EQ(vec.size(), 4);

// Test clear
vec.clear();
Expand Down
Loading