felixguendling / cista

Cista is a simple, high-performance, zero-copy C++ serialization & reflection library.
https://cista.rocks
MIT License
1.8k stars 116 forks source link

Static type hashing of std::complex #177

Open AdelKS opened 1 year ago

AdelKS commented 1 year ago

Hello,

I came across the need to serializing an std::array<std::complex<double>, N> and static type hashing doesn't have the specialization of std::complex, would be awesome to have. std::complex<T> have strict requirements where it must be equivalent to a C array of two T types, so there shouldn't be any fundamental issues.

This work as a workaround:

class A: public std::array<std::complex<double>, N>
{
public: 
   using std::array<std::complex<double>, N>::array;

  auto cista_members()
  {
    return std::tie(*reinterpret_cast<std::array<double, 2*N>*>(this));
  }
};

Thanks!

felixguendling commented 1 year ago

cista_members would probably be used prior to trying to using the aggregate structured bindings trick. But if you have another overload, this will take precedence. It should be easy to add std::complex support either by writing a custom static_type_hash function in user code or in cista itself.

AdelKS commented 1 year ago

But if you have another overload, this will take precedence.

I am not sure I understand what overload you are talking about, of cista_members() ?

It should be easy to add std::complex support either by writing a custom static_type_hash function in user code or in cista itself.

I will try that in user code meanwhile, but I think the structured binding trick on std::complex does not work because its data is protected, so serialization will not work :thinking:

felixguendling commented 1 year ago

I am not sure I understand what overload you are talking about, of cista_members() ?

I am talking about the overload of static_type_hash, not cista_members. cista_members will only be used to compute a type hash of a type if there's no other static_type_hash function available for this type.

I will try that in user code meanwhile, but I think the structured binding trick on std::complex does not work because its data is protected, so serialization will not work thinking

As you mentioned, the data layout of std::complex<X, Y> depends only on its types X and Y. So static_type_hash<std::complex<X, Y>> should yield the combination of static_type_hash<X> and static_type_hash<Y>. There is no access to any members required to compute these values.

AdelKS commented 1 year ago

I am talking about the overload of static_type_hash

Alright, yeah I do not have any overload of static_type_hash

As you mentioned, the data layout of std::complex<X, Y> depends only on its types X and Y. So static_type_hash<std::complex<X, Y>> should yield the combination of static_type_hash and static_type_hash. There is no access to any members required to compute these values.

Got it, I can write an overload of static_type_hash for std::complex<T>, no data access needed.

I meant it for serialization: even if ones fixes the static_type_hash aspect of std::complex in cista itself, I am afraid it will still need special handling for reading and writing data into an std::complex since structured binding won't cut it, that's what I mean. But I can work around that by writing a reinterpret_casted member in cista_members() of the class that uses std::complex, but what I do not understand is if I do only this last part, like I shown in the class A in my first message, I still get errors that cista is trying to hash std::complex even though it shouldn't :thinking:

Edit: as I thought, after overloading the type_hash only I get

cista/serialization.h:158:19: error: static assertion failed: Please implement custom serializer
  158 |     static_assert(to_tuple_works_v<Type>, "Please implement custom serializer");
cista/serialization.h:158:19: note: ‘cista::to_tuple_works_v<std::complex<double> >’ evaluates to false
AdelKS commented 1 year ago

My bad, my workaround with reinterpret_cast didn't work because of a mistake on my end. It does work and there's no need to overload static_type_hash for it to work. I updated my original message.