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

as() with parameter #1064

Open
wants to merge 1 commit into
base: cpp_master
Choose a base branch
from

Conversation

servantftechnicolor
Copy link

Hi,

I would like to be able to cast my msgpack::object to a custom type with some parameter.

I have a set of nested structures. At the beginning of the msgpack, i retrieve a version number. I want to be able to pass this version number recursively to ::as(...) such that they know how to react dynamically to my old structures.

It's a draft

@servantftechnicolor servantftechnicolor changed the base branch from master to cpp_master April 17, 2023 12:16
@redboltz
Copy link
Contributor

Sorry for the late reply. I'd like to know more about the usecase.
Could you write the minimal code example that demonstrate your usecase ?
In the code, please mention using comments what is the point you cannot achieve and extra as parameter solve it.

I'm thinking that pherhaps your usecase could solve user side addtional code.

@servantftechnicolor
Copy link
Author

servantftechnicolor commented Apr 20, 2023

let's say at some point i have the following structure

struct A
{
 int version = 1
 struct B
 {
  struct C
  {
   int oldx;
   int oldy;
  } c;
 } b;
} a;

then, later after some changes

struct A
{
 int version = 2
 struct B
 {
  struct C
  {
   int z (== f(oldx, oldy))
  } c;
 } b;
} a;

i don't know at runtime which version i will read. so ...

template <>
struct convert<aliceVision::sfmData::SfMData> {
    msgpack::object const& operator()(const msgpack::object &o, A & output) const 
    {
        int version = o.via.array.ptr[0] .as<int>();
        output.B = o.via.array.ptr[0] .as<B>(version);
        
        return o;
    }
};

template <>
struct convert<aliceVision::sfmData::SfMData, int> {
    msgpack::object const& operator()(const msgpack::object &o, B & output, int version) const 
    {
        output.C = o.via.array.ptr[0] .as<C>(version);
        return o;
    }
};

template <>
struct convert<aliceVision::sfmData::SfMData> {
    msgpack::object const& operator()(const msgpack::object &o, C & output) const 
    {
       if version == 1
         output.z = f(o.via.array.ptr[0] .as<int>(), o.via.array.ptr[1] .as<int>());
       if version == 2
         output.z = o.via.array.ptr[0] .as<int>();
        
       return o;
    }
};

@redboltz
Copy link
Contributor

Old A and new A cannot exist in the same program even if compiler doesn't report error. It is ODR violation.

@servantftechnicolor
Copy link
Author

No, old A does not exist, it is a previous structure which was stored in a msgpack buffer. Only the new one exist really. But i try to import the old one.

@redboltz
Copy link
Contributor

redboltz commented Apr 20, 2023

Ok. Try this:

#include <iostream>
#include <msgpack.hpp>

struct old_a {
    int version = 1;
    struct nest1 {
        struct nest2 {
            int oldx;
            int oldy;
        };
        nest2 n2;
    };
    nest1 n1;
};

inline std::ostream& operator<<(std::ostream& o, old_a const& v) {
    std::cout << "version:" << v.version << " ";
    std::cout << "oldx:" << v.n1.n2.oldx << " ";
    std::cout << "oldy:" << v.n1.n2.oldy;
    return o;
}

struct new_a {
    int version = 2;
    struct nest1 {
        struct nest2 {
            int z;
        };
        nest2 n2;
    };
    nest1 n1;
};

inline std::ostream& operator<<(std::ostream& o, new_a const& v) {
    std::cout << "version:" << v.version << " ";
    std::cout << "z:" << v.n1.n2.z;
    return o;
}
namespace msgpack {
MSGPACK_API_VERSION_NAMESPACE(MSGPACK_DEFAULT_API_NS) {
namespace adaptor {

template<>
struct pack<old_a> {
    template <typename Stream>
    packer<Stream>& operator()(msgpack::packer<Stream>& o, old_a const& v) const {
        // packing member variables as an array.
        o.pack_array(2); // for old_a
        o.pack(v.version);
        o.pack_array(1); // for nest1
        o.pack_array(2); // for nest2
        o.pack(v.n1.n2.oldx);
        o.pack(v.n1.n2.oldy);
        return o;
    }
};

template<>
struct convert<old_a> {
    msgpack::object const& operator()(msgpack::object const& o, old_a& v) const {
        if (o.type != msgpack::type::ARRAY) throw msgpack::type_error();
        if (o.via.array.size != 2) throw msgpack::type_error();
        v.version = o.via.array.ptr[0].as<int>();
        switch (v.version) {
        case 1: {
            auto on1 = o.via.array.ptr[1];
            if (on1.type != msgpack::type::ARRAY) throw msgpack::type_error();
            if (on1.via.array.size != 1) throw msgpack::type_error();

            auto on2 = on1.via.array.ptr[0];
            if (on2.type != msgpack::type::ARRAY) throw msgpack::type_error();
            if (on2.via.array.size != 2) throw msgpack::type_error();
            v.n1.n2.oldx = on2.via.array.ptr[0].as<int>();
            v.n1.n2.oldy = on2.via.array.ptr[1].as<int>();
        } break;

        default:
            throw msgpack::type_error();
        }
        return o;
    }
};

template<>
struct pack<new_a> {
    template <typename Stream>
    packer<Stream>& operator()(msgpack::packer<Stream>& o, new_a const& v) const {
        // packing member variables as an array.
        o.pack_array(2); // for old_a
        o.pack(v.version);
        o.pack_array(1); // for nest1
        o.pack_array(1); // for nest2
        o.pack(v.n1.n2.z);
        return o;
    }
};

template<>
struct convert<new_a> {
    msgpack::object const& operator()(msgpack::object const& o, new_a& v) const {
        if (o.type != msgpack::type::ARRAY) throw msgpack::type_error();
        if (o.via.array.size != 2) throw msgpack::type_error();
        auto version = o.via.array.ptr[0].as<int>();
        switch (version) {
        case 1: {
            auto on1 = o.via.array.ptr[1];
            if (on1.type != msgpack::type::ARRAY) throw msgpack::type_error();
            if (on1.via.array.size != 1) throw msgpack::type_error();

            auto on2 = on1.via.array.ptr[0];
            if (on2.type != msgpack::type::ARRAY) throw msgpack::type_error();
            if (on2.via.array.size != 2) throw msgpack::type_error();
            // f(oldx, oldy) = oldx * oldy
            v.n1.n2.z = on2.via.array.ptr[0].as<int>() * on2.via.array.ptr[1].as<int>();
        } break;

        case 2: {
            auto on1 = o.via.array.ptr[1];
            if (on1.type != msgpack::type::ARRAY) throw msgpack::type_error();
            if (on1.via.array.size != 1) throw msgpack::type_error();

            auto on2 = on1.via.array.ptr[0];
            if (on2.type != msgpack::type::ARRAY) throw msgpack::type_error();
            if (on2.via.array.size != 1) throw msgpack::type_error();
            v.n1.n2.z = on2.via.array.ptr[0].as<int>();
        } break;

        default:
            throw msgpack::type_error();
        }
        return o;
    }
};

} // namespace adaptor
} // MSGPACK_API_VERSION_NAMESPACE(MSGPACK_DEFAULT_API_NS)
} // namespace msgpack


int main() {
    old_a oa_org;
    oa_org.n1.n2.oldx =  3;
    oa_org.n1.n2.oldy =  4;
    std::cout << oa_org << std::endl;

    msgpack::sbuffer sb;
    msgpack::pack(sb, oa_org);
    auto oh = msgpack::unpack(sb.data(), sb.size());
    std::cout << *oh << std::endl;

    old_a oa = oh->as<old_a>();
    std::cout << oa << std::endl;

    new_a na = oh->as<new_a>();
    std::cout << na << std::endl;

    msgpack::sbuffer sb2;
    msgpack::pack(sb2, na);
    auto oh2 = msgpack::unpack(sb2.data(), sb2.size());
    std::cout << *oh2 << std::endl;
    new_a na2 = oh2->as<new_a>();
    std::cout << na2 << std::endl;
}

You can use non intrusive adaptor that support nest type. It can achieve your goal.
old_a::ns1 (and also old::ns1::ns2) is completely different from new_a::ns1 (and also old::ns1::ns2), so the adaptor of them cannot be reused.

If you want to avoid code repeatation, then you can introduce common class as the member of both old_a (and/or nested class) and new_a.

@servantftechnicolor
Copy link
Author

That's not a solution to me as it breaks all the per type reusability. If my version dependent structure is nested ten layers deep inside the structure hierarchy, then i have to manage the decoding inside one large function.

@redboltz
Copy link
Contributor

redboltz commented Apr 20, 2023

Unfortunately, 10 layers nested class is very unusual usecase for me. For that usecase, I don't accapt library modification.
Again, olda::type and newa::type is unreleated in general.

You can write partial covert function that takes version parameter.

@redboltz
Copy link
Contributor

I considered again. I think that your 10 layers nested case is still unusual for me, but is there other general usecase ?
I come up withe something like this:

struct {
    int my_data_format; // actually enum. 
    std::map<std::string, std::vector<my_data>> data;
};

In this case, the same issue could happen.
So your PR could solve more general isssue,hence the PR is nice to have.


I noticed some points improve your PR.

  1. msgpack-c needs to support C++03.
    C++03 version as() and convert() should have the same overload.
    https://github.com/msgpack/msgpack-c/pull/1064/files#diff-ceb18bd1c53e365d2ee418c4b3e8187605dc1ffbd755527d9ac01174c3c8b30fR110
  2. Please replace C with Hint and param with hint. I believe that it is more meaningful name.
  3. Tests are required.
  4. CI must be passed. Perhaps CI would have environmental issue. If so, please fix it. See ci: Use ubuntu-latest for CI on Linux #1061.

@redboltz
Copy link
Contributor

redboltz commented Apr 21, 2023

I noticed that bigger impact modification is requried:

The adaptors take user type as template parameters (typically T) should propagate hint.

For example, std::vector<T, Alloc>.

template <typename T, typename Alloc>
struct as<std::vector<T, Alloc>, typename std::enable_if<msgpack::has_as<T>::value>::type> {
std::vector<T, Alloc> operator()(const msgpack::object& o) const {
if (o.type != msgpack::type::ARRAY) { throw msgpack::type_error(); }
std::vector<T, Alloc> v;
v.reserve(o.via.array.size);
if (o.via.array.size > 0) {
msgpack::object* p = o.via.array.ptr;
msgpack::object* const pend = o.via.array.ptr + o.via.array.size;
do {
v.push_back(p->as<T>());

should modify as follows:

template <typename T, typename Alloc>
struct as<std::vector<T, Alloc>, typename std::enable_if<msgpack::has_as<T>::value>::type> {
    template <typename Hint> // ** added **
    std::vector<T, Alloc> operator()(const msgpack::object& o, Hint const& hint) const { // ** added **
        if (o.type != msgpack::type::ARRAY) { throw msgpack::type_error(); }
        std::vector<T, Alloc> v;
        v.reserve(o.via.array.size);
        if (o.via.array.size > 0) {
            msgpack::object* p = o.via.array.ptr;
            msgpack::object* const pend = o.via.array.ptr + o.via.array.size;
            do {
                v.push_back(p->as<T>(hint)); // ** added **

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.

2 participants