decimad / luabind-deboostified

Create Lua bindings for your C++ code easily - my improvements
http://www.vrac.iastate.edu/vancegroup/docs/luabind/
Other
71 stars 26 forks source link

const references to floats do not work with object calls #13

Closed dposluns closed 9 years ago

dposluns commented 9 years ago

The following was written in VS2013 in 64-bit:

void testFloat(const float& f, luabind::object& o)
{
    o(f);
}

static const char TEST_LUA[] =
"function testFloat(f)\n"
"    print('Out: ' .. tostring(f));\n"
"end";

int _tmain(int argc, _TCHAR* argv[])
{
    lua_State* L = lua_open();
    luaL_openlibs(L);
    int error = luaL_loadstring(L, TEST_LUA) || lua_pcall(L, 0, 0, 0);

    luabind::open(L);
    luabind::object obj = luabind::globals(L)["testFloat"];
    testFloat(1.0f, obj);

    system("pause");

    return 0;
}

This throws with "Trying to use unregistered: float const * __ptr64".

For whatever reason the float is being interpreted as a class instead of a primitive value.

Any idea how this can be fixed?

decimad commented 9 years ago

The default converter switch in native_converter.hpp doesn't match refs to native types to the native type converter. Refs to native can occur since perfect forwarding is in place. At least const-ref should be allowed for cpp-to-lua (maybe even non-const-ref), but the other way around I would like to forbid, since it cannot achieve what the user might want to try (and would mean to keep an object for the lvalue-ref around on the stack during calls). Maybe a compilation error would be best then. I will have a look.

dposluns commented 9 years ago

Perfect forwarding doesn't appear to solve it because the specialization is not being chosen when the type is a reference.

I have tried modifying native_converter.hpp to specialize on const and const& (as you do with types like bool) for integral and floating-point types and it appears to fix the problem.

    template <typename T>
    struct default_converter < T const, typename std::enable_if< std::is_integral<T>::value >::type >
        : integer_converter<T>
    {
    };

    template <typename T>
    struct default_converter < T const&, typename std::enable_if< std::is_integral<T>::value >::type >
        : integer_converter<T>
    {
    };

    template <typename T>
    struct default_converter < T const, typename std::enable_if< std::is_floating_point<T>::value >::type >
        : number_converter<T>
    {
    };

    template <typename T>
    struct default_converter < T const&, typename std::enable_if< std::is_floating_point<T>::value >::type >
        : number_converter<T>
    {
    };

Does this make sense to you?

decimad commented 9 years ago

Yeah, that's what I meant, since perfect forwarding is in place, these refs show up at the converter switch, they were swallowed by call-by-value before I believe. Anyways, ltjax was so nice as to send a pull-request with his changes, I believe this problem is amongst one of his fixes.

decimad commented 9 years ago

If it doesn't fix your problem, then I will have a look at your fix. Maybe to_cpp should be disabled in case of ref (and maybe const ref).

dposluns commented 9 years ago

I don't entirely understand what was changed but it appears to have been solved in this latest pull. Thanks!