Fix methods params handling (#74)

libs/u-test

@@ -1 +1 @@
-Subproject commit 37bf08dac653b258ceb468cbdf8fcb402ba58c3a
+Subproject commit 34ee5de696c2c12d8ded5793f2c5504f2e6ee168

src/cpp/channel.cpp

@@ -14,10 +14,12 @@ void Channel::exportAPI(sol::state_view& lua) {
     sol::stack::pop<sol::object>(lua);
 }
 
-Channel::Channel(sol::optional<int> capacity) : data_(std::make_shared<SharedData>()) {
+Channel::Channel(const sol::stack_object& capacity) : data_(std::make_shared<SharedData>()){
-    if (capacity) {
+    if (capacity.valid()) {
-        REQUIRE(capacity.value() >= 0) << "Invalid capacity value = " << capacity.value();
+        REQUIRE(capacity.get_type() == sol::type::number) << "bad argument #1 to 'effil.channel' (number expected, got "
-        data_->capacity_ = static_cast<size_t>(capacity.value());
+                                                          << luaTypename(capacity) << ")";
+        REQUIRE(capacity.as<int>() >= 0) << "effil.channel: invalid capacity value = " << capacity.as<int>();
+        data_->capacity_ = capacity.as<size_t>();
     }
     else {
         data_->capacity_ = 0;
@@ -33,12 +35,14 @@ bool Channel::push(const sol::variadic_args& args) {
         return false;
     effil::StoredArray array;
     for (const auto& arg : args) {
+        try {
             auto obj = createStoredObject(arg.get<sol::object>());
-
             addReference(obj->gcHandle());
             obj->releaseStrongReference();
             array.emplace_back(obj);
         }
+        RETHROW_WITH_PREFIX("effil.channel:push");
+    }
     if (data_->channel_.empty())
         data_->cv_.notify_one();
     data_->channel_.emplace(array);

src/cpp/channel.h

@@ -10,7 +10,7 @@ namespace effil {
 
 class Channel : public GCObject {
 public:
-    Channel(sol::optional<int> capacity);
+    Channel(const sol::stack_object& capacity);
     static void exportAPI(sol::state_view& lua);
 
     bool push(const sol::variadic_args& args);

src/cpp/garbage-collector.cpp

@@ -1,6 +1,7 @@
 #include "garbage-collector.h"
 
 #include "utils.h"
+#include "lua-helpers.h"
 
 #include <cassert>
 
@@ -64,11 +65,12 @@ sol::table GC::exportAPI(sol::state_view& lua) {
     api["pause"] = [] { instance().pause(); };
     api["resume"] = [] { instance().resume(); };
     api["enabled"] = [] { return instance().enabled(); };
-    api["step"] = [](sol::optional<int> newStep){
+    api["step"] = [](const sol::stack_object& newStep){
         auto previous = instance().step();
-        if (newStep) {
+        if (newStep.valid()) {
-            REQUIRE(*newStep <= 0) << "gc.step have to be > 0";
+            REQUIRE(newStep.get_type() == sol::type::number) << "bad argument #1 to 'effil.gc.step' (number expected, got " << luaTypename(newStep) << ")";
-            instance().step(static_cast<size_t>(*newStep));
+            REQUIRE(newStep.as<int>() >= 0) << "effil.gc.step: invalid capacity value = " << newStep.as<int>();
+            instance().step(newStep.as<size_t>());
         }
         return previous;
     };

src/cpp/lua-helpers.cpp

@@ -51,13 +51,13 @@ sol::function loadString(const sol::state_view& lua, const std::string& str) {
 std::chrono::milliseconds fromLuaTime(int duration, const sol::optional<std::string>& period) {
     using namespace std::chrono;
 
-    REQUIRE(duration >= 0) << "Invalid duration interval: " << duration;
+    REQUIRE(duration >= 0) << "invalid duration interval: " << duration;
 
     std::string metric = period ? period.value() : "s";
     if (metric == "ms") return milliseconds(duration);
     else if (metric == "s") return seconds(duration);
     else if (metric == "m") return minutes(duration);
-    else throw sol::error("invalid time identification: " + metric);
+    else throw sol::error("invalid time metric: " + metric);
 }
 
 } // namespace effil

src/cpp/lua-helpers.h

@@ -6,10 +6,31 @@
 
 namespace effil {
 
+class SharedTable;
+class Channel;
+class Thread;
+
 std::string dumpFunction(const sol::function& f);
 sol::function loadString(const sol::state_view& lua, const std::string& str);
 std::chrono::milliseconds fromLuaTime(int duration, const sol::optional<std::string>& period);
 
+template <typename SolObject>
+std::string luaTypename(const SolObject& obj) {
+    if (obj.get_type() == sol::type::userdata) {
+        if (obj.template is<SharedTable>())
+            return "effil.table";
+        else if (obj.template is<Channel>())
+            return "effil.channel";
+        else if (obj.template is<std::shared_ptr<Thread>>())
+            return "effil.thread";
+        else
+            return "userdata";
+    }
+    else {
+        return lua_typename(obj.lua_state(), (int)obj.get_type());
+    }
+}
+
 typedef std::vector<effil::StoredObject> StoredArray;
 
 } // namespace effil

src/cpp/lua-module.cpp

@@ -28,23 +28,15 @@ sol::object createTable(sol::this_state lua, const sol::optional<sol::object>& t
     return sol::make_object(lua, GC::instance().create<SharedTable>());
 }
 
-sol::object createChannel(sol::optional<int> capacity, sol::this_state lua) {
+sol::object createChannel(const sol::stack_object& capacity, sol::this_state lua) {
     return sol::make_object(lua, GC::instance().create<Channel>(capacity));
 }
 
 SharedTable globalTable = GC::instance().create<SharedTable>();
 
-std::string userdataType(const sol::object& something) {
+std::string getLuaTypename(const sol::stack_object& obj)
-    assert(something.get_type() == sol::type::userdata);
+{
-    if (something.template is<SharedTable>()) {
+    return luaTypename<>(obj);
-        return "effil.table";
-    } else if (something.template is<Channel>()) {
-        return "effil.channel";
-    } else if (something.template is<std::shared_ptr<Thread>>()) {
-        return "effil.thread";
-    } else {
-        return "userdata";
-    }
 }
 
 } // namespace
@@ -72,7 +64,7 @@ int luaopen_libeffil(lua_State* L) {
             "G", sol::make_object(lua, globalTable),
             "gc", GC::exportAPI(lua),
             "channel", createChannel,
-            "userdata_type", userdataType,
+            "type", getLuaTypename,
             "pairs", SharedTable::globalLuaPairs,
             "ipairs", SharedTable::globalLuaIPairs
     );

src/cpp/shared-table.cpp

@@ -14,6 +14,11 @@ bool isSharedTable(const SolObject& obj) {
     return obj.valid() && obj.get_type() == sol::type::userdata && obj.template is<SharedTable>();
 }
 
+template<typename SolObject>
+bool isAnyTable(const SolObject& obj) {
+    return obj.valid() && ((obj.get_type() == sol::type::userdata && obj.template is<SharedTable>()) || obj.get_type() == sol::type::table);
+}
+
 } // namespace
 
 SharedTable::SharedTable() : data_(std::make_shared<SharedData>()) {}
@@ -164,12 +169,16 @@ void SharedTable::luaNewIndex(const sol::stack_object& luaKey, const sol::stack_
             }
         }
     }
+    try {
         rawSet(luaKey, luaValue);
+    } RETHROW_WITH_PREFIX("effil.table");
 }
 
 sol::object SharedTable::luaIndex(const sol::stack_object& luaKey, sol::this_state state) {
     DEFFINE_METAMETHOD_CALL("__index", *this, luaKey)
+    try {
         return rawGet(luaKey, state);
+    } RETHROW_WITH_PREFIX("effil.table");
 }
 
 StoredArray SharedTable::luaCall(sol::this_state state, const sol::variadic_args& args) {
@@ -258,8 +267,12 @@ SharedTable::PairsIterator SharedTable::luaIPairs(sol::this_state state) {
  * Lua static API functions
  */
 
-SharedTable SharedTable::luaSetMetatable(SharedTable& stable, const sol::stack_object& mt) {
+SharedTable SharedTable::luaSetMetatable(const sol::stack_object& tbl, const sol::stack_object& mt) {
-    REQUIRE(mt.get_type() == sol::type::table || isSharedTable(mt)) << "Unexpected type of setmetatable argument";
+    REQUIRE(isAnyTable(tbl)) << "bad argument #1 to 'effil.setmetatable' (table expected, got " << luaTypename(tbl) << ")";
+    REQUIRE(isAnyTable(mt)) << "bad argument #2 to 'effil.setmetatable' (table expected, got " << luaTypename(mt) << ")";
+
+    SharedTable stable = GC::instance().get<SharedTable>(createStoredObject(tbl)->gcHandle());
+
     std::lock_guard<SpinMutex> lock(stable.data_->lock);
     if (stable.data_->metatable != GCNull) {
         stable.removeReference(stable.data_->metatable);
@@ -272,32 +285,50 @@ SharedTable SharedTable::luaSetMetatable(SharedTable& stable, const sol::stack_o
     return stable;
 }
 
-sol::object SharedTable::luaGetMetatable(const SharedTable& stable, const sol::this_state& state) {
+sol::object SharedTable::luaGetMetatable(const sol::stack_object& tbl, sol::this_state state) {
+    REQUIRE(isSharedTable(tbl)) << "bad argument #1 to 'effil.getmetatable' (effil.table expected, got " << luaTypename(tbl) << ")";
+    auto& stable = tbl.as<SharedTable>();
+
     std::lock_guard<SpinMutex> lock(stable.data_->lock);
     return stable.data_->metatable == GCNull ? sol::nil :
             sol::make_object(state, GC::instance().get<SharedTable>(stable.data_->metatable));
 }
 
-sol::object SharedTable::luaRawGet(const SharedTable& stable, const sol::stack_object& key, sol::this_state state) {
+sol::object SharedTable::luaRawGet(const sol::stack_object& tbl, const sol::stack_object& key, sol::this_state state) {
-    return stable.rawGet(key, state);
+    REQUIRE(isSharedTable(tbl)) << "bad argument #1 to 'effil.rawget' (effil.table expected, got " << luaTypename(tbl) << ")";
+    try {
+        return tbl.as<SharedTable>().rawGet(key, state);
+    } RETHROW_WITH_PREFIX("effil.rawget");
 }
 
-SharedTable SharedTable::luaRawSet(SharedTable& stable, const sol::stack_object& key, const sol::stack_object& value) {
+SharedTable SharedTable::luaRawSet(const sol::stack_object& tbl, const sol::stack_object& key, const sol::stack_object& value) {
+    REQUIRE(isSharedTable(tbl)) << "bad argument #1 to 'effil.rawset' (effil.table expected, got " << luaTypename(tbl) << ")";
+    try {
+        auto& stable = tbl.as<SharedTable>();
         stable.rawSet(key, value);
         return stable;
+    } RETHROW_WITH_PREFIX("effil.rawset");
 }
 
-size_t SharedTable::luaSize(SharedTable& stable) {
+size_t SharedTable::luaSize(const sol::stack_object& tbl) {
+    REQUIRE(isSharedTable(tbl)) << "bad argument #1 to 'effil.size' (effil.table expected, got " << luaTypename(tbl) << ")";
+    try {
+        auto& stable = tbl.as<SharedTable>();
         std::lock_guard<SpinMutex> g(stable.data_->lock);
         return stable.data_->entries.size();
+    } RETHROW_WITH_PREFIX("effil.size");
 }
 
-SharedTable::PairsIterator SharedTable::globalLuaPairs(sol::this_state state, SharedTable& obj) {
+SharedTable::PairsIterator SharedTable::globalLuaPairs(sol::this_state state, const sol::stack_object& obj) {
-    return obj.luaPairs(state);
+    REQUIRE(isSharedTable(obj)) << "bad argument #1 to 'effil.pairs' (effil.table expected, got " << luaTypename(obj) << ")";
+    auto& tbl = obj.as<SharedTable>();
+    return tbl.luaPairs(state);
 }
 
-SharedTable::PairsIterator SharedTable::globalLuaIPairs(sol::this_state state, SharedTable& obj) {
+SharedTable::PairsIterator SharedTable::globalLuaIPairs(sol::this_state state, const sol::stack_object& obj) {
-    return obj.luaIPairs(state);
+    REQUIRE(isSharedTable(obj)) << "bad argument #1 to 'effil.ipairs' (effil.table expected, got " << luaTypename(obj) << ")";
+    auto& tbl = obj.as<SharedTable>();
+    return tbl.luaIPairs(state);
 }
 
 #undef DEFFINE_METAMETHOD_CALL_0

src/cpp/shared-table.h

@@ -52,13 +52,13 @@ public:
     static sol::object luaConcat(sol::this_state, const sol::stack_object&, const sol::stack_object&);
 
     // Stand alone functions for effil::table available in Lua
-    static SharedTable luaSetMetatable(SharedTable& stable, const sol::stack_object& mt);
+    static SharedTable luaSetMetatable(const sol::stack_object& tbl, const sol::stack_object& mt);
-    static sol::object luaGetMetatable(const SharedTable& stable, const sol::this_state& state);
+    static sol::object luaGetMetatable(const sol::stack_object& tbl, const sol::this_state state);
-    static sol::object luaRawGet(const SharedTable& stable, const sol::stack_object& key, sol::this_state state);
+    static sol::object luaRawGet(const sol::stack_object& tbl, const sol::stack_object& key, sol::this_state state);
-    static SharedTable luaRawSet(SharedTable& stable, const sol::stack_object& key, const sol::stack_object& value);
+    static SharedTable luaRawSet(const sol::stack_object& tbl, const sol::stack_object& key, const sol::stack_object& value);
-    static size_t luaSize(SharedTable& stable);
+    static size_t luaSize(const sol::stack_object& tbl);
-    static PairsIterator globalLuaPairs(sol::this_state state, SharedTable& obj);
+    static PairsIterator globalLuaPairs(sol::this_state state, const sol::stack_object& obj);
-    static PairsIterator globalLuaIPairs(sol::this_state state, SharedTable& obj);
+    static PairsIterator globalLuaIPairs(sol::this_state state, const sol::stack_object& obj);
 
 private:
     PairsIterator getNext(const sol::object& key, sol::this_state lua);

src/cpp/stored-object.cpp

@@ -185,7 +185,7 @@ StoredObject fromSolObject(const SolObject& luaObject) {
             return std::make_unique<GCObjectHolder<SharedTable>>(table.handle());
         }
         default:
-            throw Exception() << "Unable to store object of that type: " << (int)luaObject.get_type() << "\n";
+            throw Exception() << "unable to store object of " << luaTypename(luaObject) << " type";
     }
     return nullptr;
 }

src/cpp/threading.cpp

@@ -212,12 +212,19 @@ void yield() {
     std::this_thread::yield();
 }
 
-void sleep(const sol::optional<int>& duration, const sol::optional<std::string>& period) {
+void sleep(const sol::stack_object& duration, const sol::stack_object& metric) {
-    if (duration)
+    if (duration.valid()) {
-        std::this_thread::sleep_for(fromLuaTime(*duration, period));
+        REQUIRE(duration.get_type() == sol::type::number) << "bad argument #1 to 'effil.sleep' (number expected, got " << luaTypename(duration) << ")";
-    else
+        if (metric.valid())
+            REQUIRE(metric.get_type() == sol::type::string) << "bad argument #2 to 'effil.sleep' (string expected, got " << luaTypename(metric) << ")";
+        try {
+            std::this_thread::sleep_for(fromLuaTime(duration.as<int>(), metric.valid() ? metric.as<std::string>() : sol::optional<std::string>()));
+        } RETHROW_WITH_PREFIX("effil.sleep");
+    }
+    else {
         yield();
     }
+}
 
 Thread::Thread(const std::string& path,
        const std::string& cpath,
@@ -236,9 +243,11 @@ Thread::Thread(const std::string& path,
     std::string strFunction = dumpFunction(function);
 
     effil::StoredArray arguments;
+    try {
         for (const auto& arg : variadicArgs) {
             arguments.emplace_back(createStoredObject(arg.get<sol::object>()));
         }
+    } RETHROW_WITH_PREFIX("effil.thread");
 
     std::thread thr(&runThread,
                     handle_,

src/cpp/threading.h

@@ -8,7 +8,7 @@ namespace effil {
 // Lua this thread API
 std::string threadId();
 void yield();
-void sleep(const sol::optional<int>&, const sol::optional<std::string>&);
+void sleep(const sol::stack_object& duration, const sol::stack_object& metric);
 
 class ThreadHandle;
 

src/cpp/utils.h

@@ -49,6 +49,7 @@ private:
 } // effil
 
 #define REQUIRE(cond) if (!(cond)) throw effil::Exception()
+#define RETHROW_WITH_PREFIX(preff) catch(const effil::Exception& err) { throw effil::Exception() << preff << ": " << err.what(); }
 
 #ifdef NDEBUG
 #define DEBUG if (false) std::cout

src/lua/effil.lua

@@ -13,19 +13,11 @@ local api = {
     G = capi.G,
     gc = capi.gc,
     channel = capi.channel,
+    type = capi.type,
     pairs = capi.pairs,
     ipairs = capi.ipairs
 }
 
-api.type = function (something)
-    local t = type(something)
-    if (t ~= "userdata") then
-        return t
-    else
-        return capi.userdata_type(something)
-    end
-end
-
 api.size = function (something)
     local t = api.type(something)
     if t == "effil.table" then
@@ -48,6 +40,10 @@ end
 --     step - who fast reacte on state changing
 --     __call - run thread, can be invoked multiple times
 api.thread = function (f)
+    if type(f) ~= "function" then
+        error("bad argument #1 to 'effil.thread' (function expected, got " .. effil.type(f) .. ")")
+    end
+
     local thread_config = {
         path = package.path,
         cpath = package.cpath,

tests/lua/gc.lua

@@ -47,7 +47,7 @@ test.gc.store_same_value = function()
         c:push(a)
     end
 
-    local c = effil.channel {}
+    local c = effil.channel()
     fill(c)
 
     c:pop()

tests/lua/run_tests

@@ -13,6 +13,7 @@ require "channel"
 require "thread"
 require "shared-table"
 require "metatable"
+require "type_mismatch"
 
 if os.getenv("STRESS") then
     require "channel-stress"

tests/lua/type_mismatch.lua

@@ -0,0 +1,156 @@
+require "bootstrap-tests"
+
+local basic_type_mismatch_test = function(err_msg, wrong_arg_num, func_name , ...)
+    local func_to_call = func_name
+    if type(func_name) == "string" then
+        func_to_call = effil
+        for word in string.gmatch(func_name, "[^%.]+") do
+            func_to_call = func_to_call[word]
+            test.is_not_nil(func_to_call)
+        end
+    end
+
+    local ret, err = pcall(func_to_call, ...)
+    test.is_false(ret)
+    print("Original error: '" .. err .. "'")
+
+    -- because error may start with trace back
+    local trunc_err = err
+    if string.len(err) > string.len(err_msg) then
+        trunc_err = string.sub(err, string.len(err) - string.len(err_msg) + 1, string.len(err))
+    end
+    test.equal(trunc_err, err_msg)
+end
+
+test.type_mismatch.input_types_mismatch = function(wrong_arg_num, expected_type, func_name, ...)
+    local args = {...}
+    local err_msg = "bad argument #" .. wrong_arg_num .. " to " ..
+            (type(func_name) == "string" and "'effil." .. func_name or func_name.name) ..
+            "' (" .. expected_type .. " expected, got " .. effil.type(args[wrong_arg_num]) .. ")"
+    basic_type_mismatch_test(err_msg, wrong_arg_num, func_name, ...)
+end
+
+test.type_mismatch.unsupported_type = function(wrong_arg_num, func_name, ...)
+    local args = {...}
+    local err_msg = (type(func_name) == "string" and "effil." ..  func_name or func_name.name)
+            .. ": unable to store object of " .. effil.type(args[wrong_arg_num]) .. " type"
+    basic_type_mismatch_test(err_msg, wrong_arg_num, func_name, ...)
+end
+
+local function generate_tests()
+    local function create_object_generator(name, func)
+        return setmetatable({ name = name }, {
+            __call = func,
+            __tostring = function() return name end
+        })
+    end
+
+    local channel_push_generator = create_object_generator("effil.channel:push",
+        function(_, ...)
+            return effil.channel():push(...)
+        end
+    )
+
+    local thread_runner_generator = create_object_generator("effil.thread",
+        function(_, ...)
+            return effil.thread(function()end)(...)
+        end
+    )
+
+    local table_set_value_generator = create_object_generator("effil.table",
+        function(_, key, value)
+            effil.table()[key] = value
+        end
+    )
+
+    local table_get_value_generator = create_object_generator("effil.table",
+        function(_, key)
+            return effil.table()[key]
+        end
+    )
+
+    local func = function()end
+    local stable = effil.table()
+    local thread = effil.thread(func)()
+    thread:wait()
+    local lua_thread = coroutine.create(func)
+
+    local all_types = { 22, "s", true, {}, stable, func, thread, effil.channel(), lua_thread }
+
+    for _, type_instance in ipairs(all_types) do
+        local typename = effil.type(type_instance)
+
+        -- effil.getmetatable
+        if typename ~= "effil.table" then
+            test.type_mismatch.input_types_mismatch(1, "effil.table", "getmetatable", type_instance)
+        end
+
+        -- effil.setmetatable
+        if typename ~= "table" and typename ~= "effil.table" then
+            test.type_mismatch.input_types_mismatch(1, "table", "setmetatable", type_instance, 44)
+            test.type_mismatch.input_types_mismatch(2, "table", "setmetatable", {}, type_instance)
+        end
+
+        if typename ~= "effil.table" then
+            -- effil.rawset
+            test.type_mismatch.input_types_mismatch(1, "effil.table", "rawset", type_instance, 44, 22)
+            -- effil.rawget
+            test.type_mismatch.input_types_mismatch(1, "effil.table", "rawget", type_instance, 44)
+            -- effil.ipairs
+            test.type_mismatch.input_types_mismatch(1, "effil.table", "ipairs", type_instance)
+            -- effil.pairs
+            test.type_mismatch.input_types_mismatch(1, "effil.table", "pairs", type_instance)
+        end
+
+        -- effil.thread
+        if typename ~= "function" then
+            test.type_mismatch.input_types_mismatch(1, "function", "thread", type_instance)
+        end
+
+        -- effil.sleep
+        if typename ~= "number" then
+            test.type_mismatch.input_types_mismatch(1, "number", "sleep", type_instance, "s")
+        end
+        if typename ~= "string" then
+            test.type_mismatch.input_types_mismatch(2, "string", "sleep", 1, type_instance)
+        end
+
+        if typename ~= "number" then
+            -- effil.channel
+            test.type_mismatch.input_types_mismatch(1, "number", "channel", type_instance)
+
+            --  effil.gc.step
+            test.type_mismatch.input_types_mismatch(1, "number", "gc.step", type_instance)
+        end
+    end
+
+    -- Below presented tests which support everything except coroutines
+
+    -- effil.rawset
+    test.type_mismatch.unsupported_type(2, "rawset", stable, lua_thread, 22)
+    test.type_mismatch.unsupported_type(3, "rawset", stable, 44, lua_thread)
+
+    -- effil.rawget
+    test.type_mismatch.unsupported_type(2, "rawget", stable, lua_thread)
+
+    -- effil.channel:push()
+    test.type_mismatch.unsupported_type(1, channel_push_generator, lua_thread)
+
+    -- effil.thread()()
+    test.type_mismatch.unsupported_type(1, thread_runner_generator, lua_thread)
+
+    -- effil.table[key] = value
+    test.type_mismatch.unsupported_type(1, table_set_value_generator, lua_thread, 2)
+    test.type_mismatch.unsupported_type(2, table_set_value_generator, 2, lua_thread)
+    -- effil.table[key]
+    test.type_mismatch.unsupported_type(1, table_get_value_generator, lua_thread)
+end
+
+-- Put it to function to limit the lifetime of objects
+generate_tests()
+
+test.type_mismatch.gc_checks_after_tests = function ()
+    collectgarbage()
+    effil.gc.collect()
+    test.equal(effil.gc.count(), 1)
+end