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

Custom serialization functions implemented in .cpp files don't work #145

Open
Yikai-Liao opened this issue Dec 15, 2023 · 7 comments
Open

Comments

@Yikai-Liao
Copy link

Yikai-Liao commented Dec 15, 2023

I want to serialize a struct without adding a serialize function in it.
So I just choose to define a serialize function outside of it.

I test the following case on windows, with vs2022 and g++ in mingw.
If I define the function totally in a header file, it will work, like this:

// A header file
struct Foo { int a; };

namespace zpp::bits {
constexpr auto serialize(auto &archive, Foo &self){
    // this is showed in terminal successfully
    std::cout << "serialize Foo" << std::endl;
    return archive(self.a);
}
}

However, if the function is just declared in the header and implemented in a cpp file, it just don't work.

// A header file
namespace zpp::bits {
constexpr auto serialize(auto &archive, Foo &self);
}
// A .cpp file, include the above header
namespace zpp::bits {
constexpr auto serialize(auto &archive, Foo &self){
    // this is not showed in terminal successfully
    std::cout << "serialize Foo" << std::endl;
    return archive(self.a);
}
}
@eyalz800
Copy link
Owner

Try to follow the example in the readme -

namespace my_namespace
{
struct person
{
    std::string name;
    int age{};
};

constexpr auto serialize(auto & archive, person & person)
{
    return archive(person.name, person.age);
}

constexpr auto serialize(auto & archive, const person & person)
{
    return archive(person.name, person.age);
}
} // namespace my_namespace

@Yikai-Liao
Copy link
Author

Yikai-Liao commented Dec 15, 2023

I tried it like this:

// in a header file

struct Foo { int a;};

constexpr auto serialize(auto& archive, const Foo& self) {
    std::cout << "serialize Foo with const " << std::endl;
    return archive(self.a);
}

constexpr auto serialize(auto &archive, Foo &self){
    std::cout << "serialize Foo without const" << std::endl;
    return archive(self.a);
}

For in and out, they both call the without const version

if I declare them in the header and implement them in a cpp file, these two implementations won't be called both.

// a header file

struct Foo { int a;};

constexpr auto serialize(auto &archive, Foo &self);
constexpr auto serialize(auto& archive, const Foo& self);
// a cpp file

constexpr auto serialize(auto& archive, const Foo& self) {
    std::cout << "serialize Foo with const " << std::endl;
    return archive(self.a);
}

constexpr auto serialize(auto &archive, Foo &self){
    std::cout << "serialize Foo without const" << std::endl;
    return archive(self.a);
}

Here is how I call them:

Foo foo{};
foo.a = 1;
vec<u8> buffer;
auto [in, out] = zpp::bits::in_out(buffer);
out(foo).or_throw();
fmt::print("{}\n", buffer);
Foo f2{};
in(f2).or_throw();

well, if I set foo to a const variable, the with const version will be called

@eyalz800
Copy link
Owner

eyalz800 commented Dec 16, 2023

I don’t think you can call a constexpr function across translation units (cpp files), it behaves like an inline function so it has to be defined for each translation unit by which it is used.

@Yikai-Liao
Copy link
Author

Yikai-Liao commented Dec 30, 2023

Well, I do find the sulution that is suitable to me. I'll show the code first:

// A header file
struct Foo {
    int a {0}; 
    Foo() = default;
    static Foo parse(std::span<const uint8_t> bytes);
    std::vector<uint8_t> dumps() const;
};
// cpp file
// include the header

namespace zpp::bits {
constexpr auto serialize(auto& archive, const Foo& data) { return archive(zpp::bits::as_bytes(data)); }
constexpr auto serialize(auto& archive, Foo& data) { return archive(zpp::bits::as_bytes(data)); }
}

Foo Foo::parse(std::span<const uint8_t> bytes) {
    auto in = zpp::bits::in(bytes);
    Foo data{};
    in(data).or_throw();
    return data;
}

std::vector<uint8_t> Food::dumps() const {
    std::vector<uint8_t> buffer;
    auto out = zpp::bits::out(buffer);
    out(data).or_throw();
    return buffer;
}

The problem is that, all the template functions or class should be defined in headers, or we need to explicitly instantialize them in .cpp files.

The signature of the serialize function is constexpr auto serialize(auto &archive, Foo &self), which means it is a template function (using auto as argument types) and we need to follow the above rule.

But I don't know the specific type of archive (clion also can't show me that). So I choose to write parse and dumps functions. And the template function serialize cound be instantialized in their defination.

In this way, I just need to call parse and dumps in other parts of my code.

There are two main advantages:

  1. I can enjoy the acceleration of parallel compilation (because this .cpp file would be a stand alone compilation unit)
  2. Decoupling the serialization implementation from the rest of the code allows for better encapsulation of the code. (Users don't need to know the implementation details of serialization, and I can change serialization methods more easily.)

@eyalz800
Copy link
Owner

You can always define in cpp instead of header but then zpp::bits won’t be able to serialize you from outside your own cpp file. The way you write the parse and dumps functions seems to suggest you are not interesting in directly serializing your class with zpp::bits from outside your cpp file anyway. No?

@Yikai-Liao
Copy link
Author

In fact, I have these classes with multiple serialization methods (saving to different standard file types, such as midi and music xml) which I want to encapsulate into a unified interface without exposing the internal implementation details.

@eyalz800
Copy link
Owner

eyalz800 commented Feb 4, 2024

You can still do that while encapsulating zpp::bits by exposing a to_byes from_byes kind of interface from your classes and use zpp::bits internally - but then you lose the ability that zpp::bits provides that it inlines everything and does so recursively.

I still hardly see the below code as non-encapsulated however -

constexpr auto serialize(auto& archive, const Foo& self) {
    return archive(self.a);
}

this code is not revealing any implementation details while placed in header - the only thing that is revealed here is that you can use a serialize function and that it calls down into an unknown archive (even the type of the archive and return value are not specified) and you don’t even need to include the header file zpp_bits in your header.

It is as much exposing implementation details as having the data members listed there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants