Returning a null smart pointer from C++ does not return a nil object about luabind-deboostified HOT 9 CLOSED

I suspect

if (!p) {
    lua_pushnil(L);
    return;
} else {
   /...
} 

in shared_ptr_converter.hpp will do the trick. Since I need to be careful with changes, I would ask you to try that out manually until I'm confident I'm not breaking anything ;)

from luabind-deboostified.

This doesn't work unfortunately, because value_converter is being used instead of shared_ptr_converter. Here's the call stack:

    luabind::detail::make_value_instance<std::shared_ptr<int> >(lua_State * L, std::shared_ptr<int> && val, std::integral_constant<bool,1> __formal) Line 107   C++
    luabind::detail::make_value_instance<std::shared_ptr<int> >(lua_State * L, std::shared_ptr<int> && val) Line 164    C++
    luabind::detail::value_converter::to_lua<std::shared_ptr<int> >(lua_State * L, std::shared_ptr<int> && x) Line 49   C++
    luabind::detail::invoke_struct<luabind::meta::type_list<>,luabind::meta::type_list<std::shared_ptr<int> >,std::shared_ptr<int> (__cdecl*)(void)>::call_struct<0,0,luabind::meta::index_list<> >::call(lua_State * L, std::shared_ptr<int> (void) * & f, std::tuple<> & argument_tuple) Line 389 C++
    luabind::detail::invoke_struct<luabind::meta::type_list<>,luabind::meta::type_list<std::shared_ptr<int> >,std::shared_ptr<int> (__cdecl*)(void)>::invoke(lua_State * L, const luabind::detail::function_object & self, luabind::detail::invoke_context & ctx, std::shared_ptr<int> (void) * & f) Line 516   C++
    luabind::detail::invoke<luabind::meta::type_list<>,luabind::meta::type_list<std::shared_ptr<int> >,std::shared_ptr<int> (__cdecl*)(void)>(lua_State * L, const luabind::detail::function_object & self, luabind::detail::invoke_context & ctx, std::shared_ptr<int> (void) * & f) Line 533  C++
    luabind::detail::function_object_impl<std::shared_ptr<int> (__cdecl*)(void),luabind::meta::type_list<std::shared_ptr<int> >,luabind::meta::type_list<> >::entry_point(lua_State * L) Line 82    C++

In invoke_struct<>::call_struct<> it references traits::result_converter, which is a value_converter instead of a shared_ptr_converter. It looks like the issue stems from how invoke_traits<Signature, PolicyList> chooses its result_converter type, but this is where it gets a little too in-the-weeds for me to follow.

from luabind-deboostified.

Okay, it was only one step to solve this issue. shared_ptr_converter.hpp is not included in default_policies.hpp, so the shared_ptr-default_converter specialization is not available for the compile. You can include it manually in the meanwhile, I will add the include to default_policies.hpp.

from luabind-deboostified.

The test also includes the converter policy manually. It might be a deliberate design decision, though I can't find a reason not to enable it by default.

from luabind-deboostified.

So, this issue extends to custom smart pointer types that are implemented by specializing get_pointer<> (as was done in the original Luabind as as still exists in the docs).

I've already observed that in this version implementing a custom smart pointer currently requires also implementing detail::has_get_pointer_::get_pointer and detail::pointer_traits<> in addition to get_pointer<>. Is adding a default_converter<> implementation then also an additional required step in order to handle this return value case?

from luabind-deboostified.

Oh I'm sorry for misleading you. That shared_ptr converter shows special behaviour that we don't really need here. It's been a while...
Okay, now to the true thing: The value_converter is actually really responsible for this, it is okay being hit, because it detects if the value supports get_ptr(). It does not check wether the pointer being held is null though (see make_instance.hpp lines 98 following).
I dont know if this is an "understandable" design setup really. Need to think about decoupling value_converter and automatic smart pointer handling... But for the time being this would be the spot.
Now I'm not really sure if it is okay to push nil there, I will check if cpp -> lua -> cpp still works as intended. But I'm off for a walk now before it gets dark... so that will take a few hours maybe.

from luabind-deboostified.

I'll give it a try as well. Looks like it should work so long as it comes at the very top of the function (before the call to get_dynamic_class which will attempt to dereference a null pointer).

from luabind-deboostified.

Okay, we're touching object_rep and instance_holder here, two of which I really want to reimplement as they're wasting space and destroy memory locality...
If only looking on the C++ side it's simple to solve, but doing class inheritance from the lua side complicates things, as object_rep really needs two phase object and payload construction. Getting this sorted out will also make it possible to implement move semantics... I hope the temporary fix you're trying holds for the moment?

from luabind-deboostified.

It appears to. I am closing the issue. Look forward to your updates. :)

from luabind-deboostified.