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

Base classes packing/converting/creating object::with_zone support in C+... #265

Merged
merged 1 commit into from Apr 28, 2015

Conversation

redboltz
Copy link
Contributor

@redboltz redboltz commented Apr 6, 2015

...+11.

@nobu-k
Copy link
Member

nobu-k commented Apr 28, 2015

I think it's perfect.

I thought MSGPACK_BASE had a problem regarding the compatibility of packed binaries at the first glance, but I noticed that your code beautifully solved it (and thanks to the frsyuki's original idea). One important characteristic of MSGPACK_DEFINE is that appending a new member at the end of it doesn't break the compatibility. For example, let's assume we have struct A.

// Version 1
struct A {
  int a;
  MSGPACK_DEFINE(a);
}

Then we add a new member b to it.

// Version 2
struct A {
  int a, b;
  MSGPACK_DEFINE(a, b);
}

The version 1 can still unpack the packed binary of the version 2's. b is simply ignored in the version 1. Then, look at the code proposed in this PR.

struct top {
    int t;
    MSGPACK_DEFINE(t);
};

struct mid1 : top {
    int m1;
    MSGPACK_DEFINE(MSGPACK_BASE(top), m1);
};

It seems like we cannot add a new member to top without breaking the compatibility of mid1. However, when packing mid1, top is serialized as an array. So, the original characteristic is preserved recursively. I like this simple solution.

nobu-k added a commit that referenced this pull request Apr 28, 2015
Base classes packing/converting/creating object::with_zone support in C+...
@nobu-k nobu-k merged commit a112ebb into msgpack:master Apr 28, 2015
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

Successfully merging this pull request may close these issues.

None yet

2 participants