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

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

Open
sagamusix opened this issue Feb 5, 2023 · 1 comment

Comments

@sagamusix
Copy link

sagamusix commented Feb 5, 2023

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
Copy link
Author

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.

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

No branches or pull requests

1 participant