p-ranav / alpaca

Serialization library written in C++17 - Pack C++ structs into a compact byte-array without any macros or boilerplate code
MIT License
462 stars 43 forks source link

Problem serializing std::tuple #22

Open mspertus opened 1 year ago

mspertus commented 1 year ago

I am not able to get tuples to serialize using g++ 11.3.0 on ubuntu 22.04.

Here's my program

#include <alpaca/alpaca.h>
#include <tuple>
std::tuple<int, double> t(1, 2.3);

std::vector<uint8_t> bytes;
auto bytes_written = alpaca::serialize(t, bytes); 

which fails to compile as follows

g++ -std=c++17 -I../build/_deps/alpaca-src/include/ test_alpaca.cpp 
In file included from ../build/_deps/alpaca-src/include/alpaca/alpaca.h:9,
                 from test_alpaca.cpp:1:
../build/_deps/alpaca-src/include/alpaca/detail/struct_nth_field.h: In instantiation of ‘constexpr decltype(auto) alpaca::detail::get(type&) [with long unsigned int index = 0; type = const std::tuple<int, double>&; long unsigned int arity = 4]’:
../build/_deps/alpaca-src/include/alpaca/alpaca.h:139:60:   required from ‘void alpaca::detail::serialize_helper(const T&, Container&, std::size_t&) [with alpaca::options O = alpaca::options::none; T = std::tuple<int, double>; long unsigned int N = 4; Container = std::vector<unsigned char>; long unsigned int I = 0; std::size_t = long unsigned int]’
../build/_deps/alpaca-src/include/alpaca/alpaca.h:156:62:   required from ‘std::size_t alpaca::serialize(const T&, Container&) [with T = std::tuple<int, double>; long unsigned int N = 4; Container = std::vector<unsigned char>; std::size_t = long unsigned int]’
test_alpaca.cpp:8:39:   required from here
../build/_deps/alpaca-src/include/alpaca/detail/struct_nth_field.h:41:11: error: 4 names provided for structured binding
   41 |     auto &[p1, p2, p3, p4] = value;
      |           ^~~~~~~~~~~~~~~~
../build/_deps/alpaca-src/include/alpaca/detail/struct_nth_field.h:41:11: note: while ‘const std::tuple<int, double>’ decomposes into 2 elements

Any thoughts appreciated. Thanks, Mike

mspertus commented 1 year ago

It looks like the problem is with it computing the wrong aggregate arity for tuple. Adding the following specialization seems to fix it (although note that there is another problem that needs to be addressed for serialization to work on g++11).

template<typename ...Ts>
struct aggregate_arity<std::tuple<Ts...>> : std::make_index_sequence<sizeof...(Ts)> {};
p-ranav commented 1 year ago

alpaca::serialize and aggregate_arity are designed for a struct input. Does it work if you wrap your tuple inside a struct? Like so:

struct MyType {
  std::tuple<int, double> t(1, 2.3);
};
mspertus commented 1 year ago

Thanks for the quick response. Unfortunately, same problem if I write it with MyType

#include <tuple>
#include <vector>
#include <cstdint>

std::vector<uint8_t> bytes;

struct MyType {
  std::tuple<int, double> t{1, 2.3};
};
MyType m;
#include <alpaca/alpaca.h>

auto bytes_written = alpaca::serialize(m.t, bytes); 

which again produces

build] /usr/bin/g++-12  -I/home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include -g -std=c++20 -fvisibility=hidden -fvisibility-inlines-hidden -MD -MT test/CMakeFiles/test_alpaca.dir/test_alpaca.cpp.o -MF test/CMakeFiles/test_alpaca.dir/test_alpaca.cpp.o.d -o test/CMakeFiles/test_alpaca.dir/test_alpaca.cpp.o -c /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/test/test_alpaca.cpp
[build] In file included from /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include/alpaca/alpaca.h:9,
[build]                  from /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/test/test_alpaca.cpp:1:
[build] /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include/alpaca/detail/struct_nth_field.h: In instantiation of ‘constexpr auto& alpaca::detail::get(type&) [with long unsigned int index = 0; type = const std::tuple<int, double>&; long unsigned int arity = 4]’:
[build] /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include/alpaca/alpaca.h:139:60:   required from ‘void alpaca::detail::serialize_helper(const T&, Container&, std::size_t&) [with alpaca::options O = alpaca::options::none; T = std::tuple<int, double>; long unsigned int N = 4; Container = std::vector<unsigned char>; long unsigned int I = 0; std::size_t = long unsigned int]’
[build] /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include/alpaca/alpaca.h:156:62:   required from ‘std::size_t alpaca::serialize(const T&, Container&) [with T = std::tuple<int, double>; long unsigned int N = 4; Container = std::vector<unsigned char>; std::size_t = long unsigned int]’
[build] /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/test/test_alpaca.cpp:16:39:   required from here
[build] /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include/alpaca/detail/struct_nth_field.h:41:11: error: 4 names provided for structured binding
[build]    41 |     auto &[p1, p2, p3, p4] = value;
[build]       |           ^~~~~~~~~~~~~~~~
[build] /home/msspertu/GitHub/high-level-enhancements-for-aws-in-cpp/build/_deps/alpaca-src/include/alpaca/detail/struct_nth_field.h:41:11: note: while ‘const std::tuple<int, double>’ decomposes into 2 elements

But this works with MyType, just like it does with a top-level tuple.

#include <alpaca/alpaca.h>
namespace alpaca::detail {
template<typename ...Ts>
struct aggregate_arity<std::tuple<Ts...>> : std::make_index_sequence<sizeof...(Ts)> {};
}

#include <tuple>
#include <vector>
#include <cstdint>

std::vector<uint8_t> bytes;

struct MyType {
  std::tuple<int, double> t{1, 2.3};
};
MyType m;

auto bytes_written = alpaca::serialize(m.t, bytes); 
p-ranav commented 1 year ago

The serialize function is expecting the struct object, not each field. If you pass in the field itself, then wrapping it in a struct does nothing.

This should work:

#include <tuple>
#include <vector>
#include <cstdint>

std::vector<uint8_t> bytes;

struct MyType {
  std::tuple<int, double> t{1, 2.3};
};
MyType m;
#include <alpaca/alpaca.h>

auto bytes_written = alpaca::serialize(m, bytes); 
mspertus commented 1 year ago

Of course. Yes, that seems to work. It also seems to resolve my other issue as well. Thanks very much!

Granting all of that, it looks like the code changes I described make top-level tuples work. Do you think that would be worth enabling? (May overlap with this issue)

p-ranav commented 1 year ago

As long as there are no regressions (unit tests), I'd be open to it, yes :)

Feel free to create a PR!

mspertus commented 1 year ago

It looks like I spoke too soon about it working if you wrap a tuple in a struct. If you modify your example to have only one type in the struct, serializing produces 0 bytes;

#include <tuple>
#include <vector>
#include <cstdint>

std::vector<uint8_t> bytes;

struct MyType {
  std::tuple<int> t{1};
};
MyType m;
#include <alpaca/alpaca.h>

auto bytes_written = alpaca::serialize(m, bytes); // bytes_written is 0

The reason for this is that the conversion from filler to tuple<int> is ambiguous due to the constructor templates in tuple, so it incorrectly deduces 0 for aggregate_arity<MyType> as shown in this godbolt.

I'll try to create a PR that fixes all tuple-related issues.