ThePhD / sol2

Sol3 (sol2 v3.0) - a C++ <-> Lua API wrapper with advanced features and top notch performance - is here, and it's great! Documentation:
http://sol2.rtfd.io/
MIT License
4.12k stars 500 forks source link

Custom containers: Default size() and set() implementations are incompatible with each other #1451

Open sagamusix opened 1 year ago

sagamusix commented 1 year ago

I'm using the current develop branch of sol. I have a custom container that is automatically detected by sol, so I did not specialize usertype_container for my container. However, deleting the last element of the container does not work as documented. Here is the current default implementation of set:

            static int set(lua_State* L_) {
                stack_object value = stack_object(L_, raw_index(3));
                if constexpr (is_linear_integral::value) {
                    // for non-associative containers,
                    // erasure only happens if it is the
                    // last index in the container
                    auto key = stack::get<K>(L_, 2);
                    auto self_size = deferred_uc::size(L_);
                    if (key == static_cast<K>(self_size)) {
                        if (type_of(L_, 3) == type::lua_nil) {
                            return erase(L_);
                        }
                    }
                }
                else {
                    if (type_of(L_, 3) == type::lua_nil) {
                        return erase(L_);
                    }
                }
                auto& self = get_src(L_);
                detail::error_result er = set_start(L_, self, stack_object(L_, raw_index(2)), std::move(value));
                return handle_errors(L_, er);
            }

Of interest is the line auto self_size = deferred_uc::size(L_);. You'd expect that if a container has 10 elements, it would return 10, but in fact it returns 1, because size returns the number of elements pushed to the Lua stack instead:

            static int size(lua_State* L_) {
                auto& self = get_src(L_);
                std::size_t r = size_start(L_, self);
                return stack::push(L_, r);
            }

As a consequence, container[#container]=nil is only doing the intended thing when #container == 1. From my understanding, size is implemented as intended, but set needs to read the container size from the stack instead of from the size return value.

sagamusix commented 1 year ago

I'm also not completely sure but shouldn't erase be called if the key isn't pointing to the last element of the container? After all that's what is being done for associative containers well. Maybe I am doing something wrong but right now I cannot do container[42] = nil from Lua because nil is not compatible with a reference to the object type in my container - and if I add an operator= (sol::optional<my_type>), sol doesn't recognize that the required assignment operator exists anymore.