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

Object conversions return the specific type that was converted. #282

Merged
merged 1 commit into from May 27, 2015

Conversation

davidchappelle
Copy link

This allows us to easily make function calls that have parameters that are dependent
upon converting from msgpack objects to concrete types. For example:

void process_args(std::tuple<int,std::string>& args)
{
    ...
}

process_args(obj.convert(std::make_tuple(100,"hello")))

You can get similar behavior by using obj.as<...>() but its performance is highly
dependent upon the specific compiler and whether or not r-value references are
supported.

This allows us to easily make function calls that have parameters that are dependent
upon converting from msgpack objects to concrete types. For example:

    void process_args(std::tuple<int,std::string>& args)
    {
        ...
    }

    process_args(obj.convert(std::make_tuple(100,"hello")))

You can get similar behavior by using obj.as<...>() but its performance is highly
dependent upon the specific compiler and whether or not r-value references are
supported.
@redboltz
Copy link
Contributor

Thank you for sending the PR.
It seems that the following code wouldn't compiled successfully.

obj.convert(std::make_tuple(100,"hello"))

The member function 'convert' has a reference parameter. It is a what we call out parameter.

inline T& object::convert(T& v) const

std::make_tuple(100,"hello") creates a temporary rvalue object. That kind of object cannot be passed as a lvalue reference parameter.

Could you check your code example and the contents of PR?

@davidchappelle
Copy link
Author

Apologies for the bad example. The existing code has the same issue regardless of my patch. Here is an updated example:

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

void process_args(std::tuple<int,std::string>& args)
{
    std::cout << "processing args" << std::endl;
}

int main(int argc, char** argv)
{
    msgpack::object obj;
    std::tuple<int, std::string> t;
    process_args(obj.convert(t));

    return 0;
}

@davidchappelle
Copy link
Author

Sounds reasonable?

@redboltz
Copy link
Contributor

Yes. We can write as follows:

    std::tuple<int, std::string> t;
    process_args(obj.convert(t));

instead of

    std::tuple<int, std::string> t;
    obj.convert(t);
    process_args(t);

I think it is a same mannar as memcpy.
http://www.cplusplus.com/reference/cstring/memcpy/

I will merge your PR. Before I do that though, could you add some tests?

I think that the following file is an appropriate location:
https://github.com/msgpack/msgpack-c/blob/master/test/convert.cpp

@redboltz redboltz merged commit 9d4da1a into msgpack:master May 27, 2015
@redboltz
Copy link
Contributor

I added tests and merged.

@redboltz
Copy link
Contributor

This fix includes very subtle breaking change.
If the client code has the following code:

void foo() {
    msgpack::object obj;
    int i;
    return obj.convert(i); // void function can return 

a compile would report the error like "cannot convert from int& to void."
However, it is very rare that returning void return code explicitly.

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