Skip to content

Commit

Permalink
potential bug fix in tuple
Browse files Browse the repository at this point in the history
  • Loading branch information
@drexlerd
drexlerd committed Aug 28, 2024
1 parent b6224b7 commit d5ac023
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 36 deletions.
24 changes: 16 additions & 8 deletions include/flatmemory/details/types/tuple.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ class Layout<Tuple<Ts...>>
{
private:
template<size_t... Is>
static consteval std::array<size_t, sizeof...(Ts) + 1> calculate_data_positions(std::index_sequence<Is...> index_sequence, size_t first_pos)
static consteval std::array<size_t, sizeof...(Ts) + 1> calculate_offset_positions(std::index_sequence<Is...> index_sequence, size_t first_pos)
{
std::array<size_t, sizeof...(Ts) + 1> data_positions {};
std::array<size_t, sizeof...(Ts) + 1> offset_positions {};
(
[&]
{
data_positions[Is] = first_pos;
offset_positions[Is] = first_pos;

using T = std::tuple_element_t<Is, std::tuple<Ts...>>;
if constexpr (IsTriviallyCopyable<T>)
Expand All @@ -76,17 +76,17 @@ class Layout<Tuple<Ts...>>
}(),
...);

data_positions[sizeof...(Ts)] = first_pos;
offset_positions[sizeof...(Ts)] = first_pos;

return data_positions;
return offset_positions;
}

public:
static constexpr size_t size = sizeof...(Ts);

static constexpr size_t buffer_size_position = 0;
static constexpr std::array<size_t, sizeof...(Ts) + 1> offset_positions =
calculate_data_positions(std::make_index_sequence<sizeof...(Ts)> {}, sizeof(BufferSizeType));
calculate_offset_positions(std::make_index_sequence<sizeof...(Ts)> {}, sizeof(BufferSizeType));

constexpr void print() const
{
Expand Down Expand Up @@ -127,6 +127,7 @@ class Builder<Tuple<Ts...>> : public IBuilder<Builder<Tuple<Ts...>>>
/* Write the data inline. */
out.write(offset_pos, m_value);

// data pos stays the same because data written in the offset pos.
return data_pos;
}

Expand Down Expand Up @@ -205,9 +206,16 @@ class Builder<Tuple<Ts...>> : public IBuilder<Builder<Tuple<Ts...>>>
template<size_t... Is>
size_t finish_iterative_impl(std::index_sequence<Is...>, size_t pos, ByteBuffer& out)
{
size_t data_pos = Layout<Tuple<Ts...>>::offset_positions.back();
size_t data_pos = pos + Layout<Tuple<Ts...>>::offset_positions.back();

([&] { data_pos = std::get<Is>(m_data).finish(Layout<Tuple<Ts...>>::offset_positions[Is], data_pos, out); }(), ...);
(
[&]
{
// std::cout << "finish_iterative_impl: offset_pos=" << pos + Layout<Tuple<Ts...>>::offset_positions[Is] << " data_pos=" << data_pos <<
// std::endl;
data_pos = std::get<Is>(m_data).finish(pos + Layout<Tuple<Ts...>>::offset_positions[Is], data_pos, out);
}(),
...);

/* Write size of the buffer to the beginning. */
out.write(pos + Layout<Tuple<Ts...>>::buffer_size_position, static_cast<BufferSizeType>(data_pos));
Expand Down
9 changes: 3 additions & 6 deletions include/flatmemory/details/types/vector.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,10 @@ class Builder<Vector<T>> : public IBuilder<Builder<Vector<T>>>
/* Write the vector size */
out.write(pos + Layout<Vector<T>>::vector_size_position, m_data.size());

size_t data_pos = Layout<Vector<T>>::vector_data_position;

/* Write the offset inline and data at offset. */
size_t offset_pos = data_pos;
size_t offset_pos = Layout<Vector<T>>::vector_data_position;
/* Write sufficiently much padding before the data. */
data_pos = offset_pos + m_data.size() * sizeof(OffsetType);
size_t data_pos = offset_pos + m_data.size() * sizeof(OffsetType);

/* Set data pos after the offset locations. */
for (size_t i = 0; i < m_data.size(); ++i)
Expand Down Expand Up @@ -636,7 +634,6 @@ class ConstView<Vector<T>> : public IConstView<ConstView<Vector<T>>>
ConstView& operator=(const View<Vector<T>>& view)
{
m_buf = view.buffer();
assert(m_buf);
return *this;
}

Expand Down Expand Up @@ -675,7 +672,7 @@ class ConstView<Vector<T>> : public IConstView<ConstView<Vector<T>>>
ConstIterator() : m_buf(nullptr) {}
ConstIterator(const uint8_t* buf) : m_buf(buf) {}

value_type operator*() const { return read_value<T>(m_buf); }
T operator*() const { return read_value<T>(m_buf); }
ConstIterator& operator++()
{
m_buf += sizeof(T);
Expand Down
67 changes: 64 additions & 3 deletions tests/unit/types/tuple.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,30 @@ TEST(FlatmemoryTests, TypesTupleStructTest)
EXPECT_EQ(view.get_buffer_size(), 36);
}

/*
struct FlatSimpleEffect
{
bool is_negated;
size_t atom_id;
}; // __attribute__((packed)); // Pack the struct to avoid padding

struct Static
{
};
struct Fluent
{
};
struct Derived
{
};

template<typename Tag = void>
using FlatBitsetLayout = flatmemory::Bitset<uint64_t, Tag>;
template<typename Tag = void>
using FlatBitsetBuilder = flatmemory::Builder<FlatBitsetLayout<Tag>>;
template<typename Tag = void>
using FlatBitset = flatmemory::ConstView<FlatBitsetLayout<Tag>>;
using FlatIndexListLayout = flatmemory::Vector<uint32_t>;

static_assert(sizeof(FlatSimpleEffect) == 16);

TEST(FlatmemoryTests, TypesTupleConditionalEffectTest)
Expand All @@ -329,7 +346,19 @@ TEST(FlatmemoryTests, TypesTupleConditionalEffectTest)
EXPECT_FALSE(effect.is_negated);
EXPECT_EQ(effect.atom_id, 4);

using FlatIndexListLayout = flatmemory::Vector<uint32_t>;
using FlatStripsActionPreconditionLayout = flatmemory::Tuple<FlatBitsetLayout<Static>, //
FlatBitsetLayout<Static>,
FlatBitsetLayout<Fluent>,
FlatBitsetLayout<Fluent>,
FlatBitsetLayout<Derived>,
FlatBitsetLayout<Derived>>;
using FlatStripsActionPreconditionBuilder = flatmemory::Builder<FlatStripsActionPreconditionLayout>;
using FlatStripsActionPrecondition = flatmemory::ConstView<FlatStripsActionPreconditionLayout>;

using FlatStripsActionEffectLayout = flatmemory::Tuple<FlatBitsetLayout<Fluent>, // add effects
FlatBitsetLayout<Fluent>>; // delete effects
using FlatStripsActionEffectBuilder = flatmemory::Builder<FlatStripsActionEffectLayout>;
using FlatStripsActionEffect = flatmemory::ConstView<FlatStripsActionEffectLayout>;

using FlatConditionalEffectLayout = flatmemory::Tuple<FlatIndexListLayout, // Positive static atom indices
FlatIndexListLayout, // Negative static atom indices
Expand All @@ -338,6 +367,28 @@ TEST(FlatmemoryTests, TypesTupleConditionalEffectTest)
FlatIndexListLayout, // Positive derived atom indices
FlatIndexListLayout, // Negative derived atom indices
FlatSimpleEffect>; // simple add or delete effect
using FlatConditionalEffectBuilder = flatmemory::Builder<FlatConditionalEffectLayout>;
using FlatConditionalEffect = flatmemory::ConstView<FlatConditionalEffectLayout>;

using FlatConditionalEffectsLayout = flatmemory::Vector<FlatConditionalEffectLayout>; // simple add or delete effect
using FlatConditionalEffectsBuilder = flatmemory::Builder<FlatConditionalEffectsLayout>;
using FlatConditionalEffects = flatmemory::ConstView<FlatConditionalEffectsLayout>;

using FlatSimpleEffectVectorLayout = flatmemory::Vector<FlatSimpleEffect>;
using FlatSimpleEffectVectorBuilder = flatmemory::Builder<FlatSimpleEffectVectorLayout>;
using FlatSimpleEffectVector = flatmemory::ConstView<FlatSimpleEffectVectorLayout>;

using FlatActionLayout = flatmemory::Tuple<uint32_t, //
double,
int*,
// FlatObjectListLayout,
FlatStripsActionPreconditionLayout
// FlatStripsActionEffectLayout,
// FlatConditionalEffectsLayout
>;
using FlatActionBuilder = flatmemory::Builder<FlatActionLayout>;
using FlatAction = flatmemory::ConstView<FlatActionLayout>;
using FlatActionVector = flatmemory::VariableSizedTypeVector<FlatActionLayout>;

Layout<FlatConditionalEffectLayout>().print();

Expand All @@ -355,7 +406,17 @@ TEST(FlatmemoryTests, TypesTupleConditionalEffectTest)

EXPECT_FALSE(view.get<6>().is_negated);
EXPECT_EQ(view.get<6>().atom_id, 4);

int* x = reinterpret_cast<int*>(100);

std::cout << "asd" << std::endl;
auto action_builder = FlatActionBuilder();
action_builder.get<0>() = 42;
action_builder.get<1>() = 0.;
action_builder.get<2>() = x;
action_builder.finish();
auto flat_action = FlatAction(action_builder.get_buffer().data());
flatmemory::print(flat_action.get_buffer(), flat_action.get_buffer_size());
}
*/

}
67 changes: 48 additions & 19 deletions tests/unit/types/vector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,28 +291,57 @@ TEST(FlatmemoryTests, TypesVectorVectorTest)

TEST(FlatmemoryTests, TypesVectorViewIteratorTest)
{
using VectorVectorUint32Layout = Vector<Vector<uint32_t>>;
using VectorVectorUint32Builder = Builder<VectorVectorUint32Layout>;
using VectorVectorUint32 = View<VectorVectorUint32Layout>;
using ConstVectorVectorUint32 = ConstView<VectorVectorUint32Layout>;

auto builder = VectorVectorUint32Builder();
builder.resize(3);
builder[0].resize(3);
builder[1].resize(4);
builder[2].resize(5);
builder.finish();

auto view = VectorVectorUint32(builder.get_buffer().data());
for (const auto element : view)
{
EXPECT_GT(element.size(), 0);
// 1D
using VectorUint32Layout = Vector<uint32_t>;
using VectorUint32Builder = Builder<VectorUint32Layout>;
using VectorUint32 = View<VectorUint32Layout>;
using ConstVectorUint32 = ConstView<VectorUint32Layout>;

auto builder = VectorUint32Builder();
builder.resize(3);
builder[0] = 5;
builder[1] = 6;
builder[2] = 7;
builder.finish();

auto view = VectorUint32(builder.get_buffer().data());
for (const auto element : view)
{
EXPECT_GT(element, 0);
}

auto const_view = ConstVectorUint32(builder.get_buffer().data());
for (const auto element : const_view)
{
EXPECT_GT(element, 0);
}
}

auto const_view = ConstVectorVectorUint32(builder.get_buffer().data());
for (const auto element : const_view)
{
EXPECT_GT(element.size(), 0);
// 2D
using VectorVectorUint32Layout = Vector<Vector<uint32_t>>;
using VectorVectorUint32Builder = Builder<VectorVectorUint32Layout>;
using VectorVectorUint32 = View<VectorVectorUint32Layout>;
using ConstVectorVectorUint32 = ConstView<VectorVectorUint32Layout>;

auto builder = VectorVectorUint32Builder();
builder.resize(3);
builder[0].resize(3);
builder[1].resize(4);
builder[2].resize(5);
builder.finish();

auto view = VectorVectorUint32(builder.get_buffer().data());
for (const auto element : view)
{
EXPECT_GT(element.size(), 0);
}

auto const_view = ConstVectorVectorUint32(builder.get_buffer().data());
for (const auto element : const_view)
{
EXPECT_GT(element.size(), 0);
}
}
}

Expand Down

0 comments on commit d5ac023

Please sign in to comment.