Skip to content

Commit

Permalink
Fix edge case with invoking functors on cached selectors with no retu…
Browse files Browse the repository at this point in the history 
…rn value.
  • Loading branch information
@jeremyong
jeremyong committed Oct 29, 2014
1 parent 722a5e9 commit 0be4e4e
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 44 deletions.
90 changes: 46 additions & 44 deletions include/selene/Selector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ class Selector {
// Functor is stored when the () operator is invoked. The argument
// is used to indicate how many return values are expected
using Functor = std::function<void(int)>;
mutable std::unique_ptr<Functor> _functor;
mutable Functor _functor;

Selector(lua_State *s, Registry &r, const std::string &name,
std::vector<Fun> traversal, Fun get, PFun put)
: _state(s), _registry(r), _name(name), _traversal{traversal},
_get(get), _put(put), _functor{nullptr} {}
_get(get), _put(put) {}

Selector(lua_State *s, Registry &r, const char *name)
: _state(s), _registry(r), _name(name), _functor{nullptr} {
: _state(s), _registry(r), _name(name) {
_get = [this, name]() {
lua_getglobal(_state, name);
};
Expand Down Expand Up @@ -73,16 +73,18 @@ class Selector {
: _state(other._state),
_registry(other._registry),
_name{other._name},
_traversal{other._traversal},
_get{other._get},
_put{other._put} {}
_traversal{other._traversal},
_get{other._get},
_put{other._put},
_functor(other._functor)
{}

~Selector() {
// If there is a functor present, execute it and collect no args
if (_functor != nullptr) {
// If there is a functor is not empty, execute it and collect no args
if (_functor) {
_traverse();
_get();
(*_functor)(0);
_functor(0);
}
lua_settop(_state, 0);
}
Expand All @@ -91,15 +93,15 @@ class Selector {
bool operator==(Selector &other) = delete;

template <typename... Args>
const Selector& operator()(Args... args) const {
const Selector operator()(Args... args) const {
auto tuple_args = std::make_tuple(std::forward<Args>(args)...);
constexpr int num_args = sizeof...(Args);
auto tmp = new Functor([this, tuple_args, num_args](int num_ret) {
detail::_push(_state, tuple_args);
lua_call(_state, num_args, num_ret);
});
_functor.reset(std::move(tmp));
return *this;
Selector copy{*this};
copy._functor = [this, tuple_args, num_args](int num_ret) {
detail::_push(_state, tuple_args);
lua_call(_state, num_args, num_ret);
};
return copy;
}

template <typename L>
Expand Down Expand Up @@ -212,17 +214,17 @@ class Selector {
std::tuple<Ret...> GetTuple() const {
_traverse();
_get();
(*_functor)(sizeof...(Ret));
_functor(sizeof...(Ret));
return detail::_pop_n_reset<Ret...>(_state);
}

template <typename T>
operator T&() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<T*>{}, _state);
lua_settop(_state, 0);
Expand All @@ -233,9 +235,9 @@ class Selector {
operator T*() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<T*>{}, _state);
lua_settop(_state, 0);
Expand All @@ -245,9 +247,9 @@ class Selector {
operator bool() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<bool>{}, _state);
lua_settop(_state, 0);
Expand All @@ -257,9 +259,9 @@ class Selector {
operator int() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<int>{}, _state);
lua_settop(_state, 0);
Expand All @@ -269,9 +271,9 @@ class Selector {
operator unsigned int() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<unsigned int>{}, _state);
lua_settop(_state, 0);
Expand All @@ -281,9 +283,9 @@ class Selector {
operator lua_Number() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<lua_Number>{}, _state);
lua_settop(_state, 0);
Expand All @@ -293,9 +295,9 @@ class Selector {
operator std::string() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<std::string>{}, _state);
lua_settop(_state, 0);
Expand All @@ -306,9 +308,9 @@ class Selector {
operator sel::function<R(Args...)>() {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<sel::function<R(Args...)>>{},
_state);
Expand Down Expand Up @@ -389,9 +391,9 @@ class Selector {
std::string ToString() const {
_traverse();
_get();
if (_functor != nullptr) {
(*_functor)(1);
_functor.reset();
if (_functor) {
_functor(1);
_functor = nullptr;
}
auto ret = detail::_pop(detail::_id<std::string>{}, _state);
lua_settop(_state, 0);
Expand Down
3 changes: 3 additions & 0 deletions test/Test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ static TestMap tests = {
{"test_set_nested_index", test_set_nested_index},
{"test_create_table_field", test_create_table_field},
{"test_create_table_index", test_create_table_index},
{"test_cache_selector_field_assignment", test_cache_selector_field_assignment},
{"test_cache_selector_field_access", test_cache_selector_field_access},
{"test_cache_selector_function", test_cache_selector_function},

{"test_register_class", test_register_class},
{"test_get_member_variable", test_get_member_variable},
Expand Down
19 changes: 19 additions & 0 deletions test/selector_tests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,22 @@ bool test_create_table_index(sel::State &state) {
state["new_table"][3] = 4;
return state["new_table"][3] == 4;
}

bool test_cache_selector_field_assignment(sel::State &state) {
sel::Selector s = state["new_table"][3];
s = 4;
return state["new_table"][3] == 4;
}

bool test_cache_selector_field_access(sel::State &state) {
state["new_table"][3] = 4;
sel::Selector s = state["new_table"][3];
return s == 4;
}

bool test_cache_selector_function(sel::State &state) {
state.Load("../test/test.lua");
sel::Selector s = state["set_global"];
s();
return state["global1"] == 8;
}
4 changes: 4 additions & 0 deletions test/test.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,8 @@ co = coroutine.create(resumable)
function resume_co()
ran, value = coroutine.resume(co)
return value
end

function set_global()
global1 = 8
end

0 comments on commit 0be4e4e

Please sign in to comment.