Fix metatable serialization/deserialization and __eq operator logic (#144)

src/cpp/function.cpp

@@ -3,6 +3,15 @@
 namespace effil {
 
 Function::Function(const sol::function& luaObject) {
+    SolTableToShared visited;
+    construct(luaObject, visited);
+}
+
+Function::Function(const sol::function& luaObject, SolTableToShared& visited) {
+    construct(luaObject, visited);
+}
+
+void Function::construct(const sol::function& luaObject, SolTableToShared& visited) {
     assert(luaObject.valid());
     assert(luaObject.get_type() == sol::type::function);
 
@@ -39,7 +48,7 @@ Function::Function(const sol::function& luaObject) {
         StoredObject storedObject;
         try {
             const auto& upvalue = sol::stack::pop<sol::object>(state);
-            storedObject = createStoredObject(upvalue);
+            storedObject = createStoredObject(upvalue, visited);
             assert(storedObject.get() != nullptr);
         }
         catch(const std::exception& err) {

src/cpp/function.h

@@ -24,7 +24,9 @@ public:
 private:
     using Converter = std::function<sol::object(const StoredObject&)>;
     sol::object convert(lua_State* state, const Converter& clbk) const;
+    void construct(const sol::function& luaObject, SolTableToShared& visited);
 
+    explicit Function(const sol::function& luaObject, SolTableToShared& visited);
     explicit Function(const sol::function& luaObject);
     friend class GC;
 };

src/cpp/shared-table.cpp

@@ -109,6 +109,12 @@ sol::object SharedTable::luaDump(sol::this_state state, BaseHolder::DumpCache& c
             result.set(pair.first->convertToLua(state, cache),
                        pair.second->convertToLua(state, cache));
         }
+        if (ctx_->metatable) {
+            const auto mt = GC::instance().get<SharedTable>(ctx_->metatable);
+            lock.unlock();
+
+            result[sol::metatable_key] = mt.luaDump(state, cache);
+        }
         return result;
     }
     return sol::table(state.L, sol::ref_index(iter->second));
@@ -134,7 +140,7 @@ sol::object SharedTable::luaDump(sol::this_state state, BaseHolder::DumpCache& c
 #define PROXY_METAMETHOD_IMPL(tableMethod, methodName, errMsg) \
     sol::object SharedTable:: tableMethod(sol::this_state state, \
             const sol::stack_object& leftObject, const sol::stack_object& rightObject) { \
-        return basicMetaMethod(methodName, errMsg, state, leftObject, rightObject); \
+        return basicBinaryMetaMethod(methodName, errMsg, state, leftObject, rightObject); \
     }
 
 namespace {
@@ -143,7 +149,7 @@ const std::string COMPARE_ERR_MSG = "attempt to compare a effil::table value";
 const std::string CONCAT_ERR_MSG = "attempt to concatenate a effil::table value";
 }
 
-sol::object SharedTable::basicMetaMethod(const std::string& metamethodName, const std::string& errMsg,
+sol::object SharedTable::basicBinaryMetaMethod(const std::string& metamethodName, const std::string& errMsg,
             sol::this_state state, const sol::stack_object& leftObject, const sol::stack_object& rightObject) {
     if (isSharedTable(leftObject)) {
         SharedTable table = leftObject.as<SharedTable>();
@@ -167,7 +173,25 @@ PROXY_METAMETHOD_IMPL(luaMod, "__mod", ARITHMETIC_ERR_MSG)
 PROXY_METAMETHOD_IMPL(luaPow, "__pow", ARITHMETIC_ERR_MSG)
 PROXY_METAMETHOD_IMPL(luaLe, "__le", ARITHMETIC_ERR_MSG)
 PROXY_METAMETHOD_IMPL(luaLt, "__lt", ARITHMETIC_ERR_MSG)
-PROXY_METAMETHOD_IMPL(luaEq, "__eq", ARITHMETIC_ERR_MSG)
+
+sol::object SharedTable::luaEq(sol::this_state state, const sol::stack_object& leftObject,
+                               const sol::stack_object& rightObject) {
+    if (isSharedTable(leftObject) && isSharedTable(rightObject)) {
+        {
+            SharedTable table = leftObject.as<SharedTable>();
+            auto ctx_ = table.ctx_;
+            DEFFINE_METAMETHOD_CALL("__eq", table, rightObject)
+        }
+        {
+            SharedTable table = rightObject.as<SharedTable>();
+            auto ctx_ = table.ctx_;
+            DEFFINE_METAMETHOD_CALL("__eq", leftObject, table)
+        }
+        const bool isEqual = leftObject.as<SharedTable>().handle() == rightObject.as<SharedTable>().handle();
+        return sol::make_object(state, isEqual);
+    }
+    return sol::make_object(state, false);
+}
 
 sol::object SharedTable::luaUnm(sol::this_state state) {
     DEFFINE_METAMETHOD_CALL_0("__unm")
@@ -304,27 +328,39 @@ SharedTable::PairsIterator SharedTable::luaIPairs(sol::this_state state) {
                 sol::make_object(state, *this));
 }
 
+SharedTable SharedTable::setMetatable(const sol::optional<SharedTable>& metaTable) {
+    UniqueLock lock(ctx_->lock);
+    if (ctx_->metatable != GCNull) {
+        ctx_->removeReference(ctx_->metatable);
+        ctx_->metatable = GCNull;
+    }
+
+    if (metaTable) {
+        ctx_->metatable = metaTable->handle();
+        ctx_->addReference(ctx_->metatable);
+    }
+    return *this;
+}
+
 /*
  * Lua static API functions
  */
 
 SharedTable SharedTable::luaSetMetatable(const sol::stack_object& tbl, const sol::stack_object& mt) {
-    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) << ")";
+    REQUIRE(isAnyTable(tbl))
+        << "bad argument #1 to 'effil.setmetatable' (table expected, got "
+        << luaTypename(tbl) << ")";
+    REQUIRE(isAnyTable(mt) || !mt.valid())
+        << "bad argument #2 to 'effil.setmetatable' (table or nil expected, got "
+        << luaTypename(mt) << ")";
 
-    SharedTable stable = GC::instance().get<SharedTable>(createStoredObject(tbl)->gcHandle());
-
-    UniqueLock lock(stable.ctx_->lock);
-    if (stable.ctx_->metatable != GCNull) {
-        stable.ctx_->removeReference(stable.ctx_->metatable);
-        stable.ctx_->metatable = GCNull;
+    SolTableToShared cache;
+    SharedTable table = GC::instance().get<SharedTable>(createStoredObject(tbl, cache)->gcHandle());
+    sol::optional<SharedTable> metatable;
+    if (mt.valid()) {
+        metatable = GC::instance().get<SharedTable>(createStoredObject(mt, cache)->gcHandle());
     }
-
-    const auto mtObj = createStoredObject(mt);
-    stable.ctx_->metatable = mtObj->gcHandle();
-    stable.ctx_->addReference(stable.ctx_->metatable);
-
-    return stable;
+    return table.setMetatable(metatable);
 }
 
 sol::object SharedTable::luaGetMetatable(const sol::stack_object& tbl, sol::this_state state) {

src/cpp/shared-table.h

@@ -35,8 +35,10 @@ public:
     void rawSet(const sol::stack_object& luaKey, const sol::stack_object& luaValue);
     sol::object get(const StoredObject& key, sol::this_state state) const;
     sol::object rawGet(const sol::stack_object& key, sol::this_state state) const;
-    static sol::object basicMetaMethod(const std::string&, const std::string&, sol::this_state,
+    static sol::object basicBinaryMetaMethod(
+            const std::string&, const std::string&, sol::this_state,
             const sol::stack_object&, const sol::stack_object&);
+    SharedTable setMetatable(const sol::optional<SharedTable>& metaTable);
 
     // These functions are metamethods available in Lua
     void luaNewIndex(const sol::stack_object& luaKey, const sol::stack_object& luaValue, sol::this_state);

src/cpp/stored-object.cpp

@@ -118,42 +118,35 @@ public:
     }
 };
 
-// This class is used as a storage for visited sol::tables
-// TODO: try to use map or unordered map instead of linear search in vector
-// TODO: Trick is - sol::object has only operator==:/
-typedef std::vector<std::pair<sol::object, GCHandle>> SolTableToShared;
+void dumpTable(SharedTable& target, const sol::table& luaTable, SolTableToShared& visited);
 
-void dumpTable(SharedTable* target, sol::table luaTable, SolTableToShared& visited);
-
-StoredObject makeStoredObject(sol::object luaObject, SolTableToShared& visited) {
+StoredObject makeStoredObject(const sol::object& luaObject, SolTableToShared& visited) {
     if (luaObject.get_type() == sol::type::table) {
         sol::table luaTable = luaObject;
-        auto comparator = [&luaTable](const std::pair<sol::table, GCHandle>& element) {
-            return element.first == luaTable;
-        };
-        auto st = std::find_if(visited.begin(), visited.end(), comparator);
-
-        if (st == std::end(visited)) {
+        auto st = std::find_if(visited.begin(), visited.end(), [&](const auto& pair) {
+            return pair.first == luaObject;
+        });
+        if (st == visited.end()) {
             SharedTable table = GC::instance().create<SharedTable>();
-            visited.emplace_back(std::make_pair(luaTable, table.handle()));
-            dumpTable(&table, luaTable, visited);
+            visited.push_back({luaTable, table.handle()});
+            dumpTable(table, luaTable, visited);
             return std::make_unique<SharedTableHolder>(table.handle());
         } else {
             return std::make_unique<SharedTableHolder>(st->second);
         }
     } else {
-        return createStoredObject(luaObject);
+        return createStoredObject(luaObject, visited);
     }
 }
 
-void dumpTable(SharedTable* target, sol::table luaTable, SolTableToShared& visited) {
+void dumpTable(SharedTable& target, const sol::table& luaTable, SolTableToShared& visited) {
     for (auto& row : luaTable) {
-        target->set(makeStoredObject(row.first, visited), makeStoredObject(row.second, visited));
+        target.set(makeStoredObject(row.first, visited), makeStoredObject(row.second, visited));
     }
 }
 
 template <typename SolObject>
-StoredObject fromSolObject(const SolObject& luaObject) {
+StoredObject fromSolObject(const SolObject& luaObject, SolTableToShared& visited) {
     switch (luaObject.get_type()) {
         case sol::type::nil:
             return std::make_unique<NilHolder>();
@@ -190,20 +183,34 @@ StoredObject fromSolObject(const SolObject& luaObject) {
             else
                 throw Exception() << "Unable to store userdata object";
         case sol::type::function: {
-            Function func = GC::instance().create<Function>(luaObject);
+            Function func = GC::instance().create<Function>(luaObject, visited);
             return std::make_unique<FunctionHolder>(func.handle());
         }
         case sol::type::table: {
             sol::table luaTable = luaObject;
+
+            const auto iter = std::find_if(visited.begin(), visited.end(), [&](const auto& pair) {
+                return pair.first == luaTable;
+            });
+            if (iter != visited.end()) {
+                return std::make_unique<SharedTableHolder>(iter->second);
+            }
             // Tables pool is used to store tables.
             // Right now not defiantly clear how ownership between states works.
             SharedTable table = GC::instance().create<SharedTable>();
-            SolTableToShared visited{{luaTable, table.handle()}};
+            visited.push_back({luaTable, table.handle()});
 
             // Let's dump table and all subtables
             // SolTableToShared is used to prevent from infinity recursion
             // in recursive tables
-            dumpTable(&table, luaTable, visited);
+            dumpTable(table, luaTable, visited);
+
+            const sol::table luaMetatable = luaTable[sol::metatable_key];
+            if (luaMetatable.valid()) {
+                SharedTable metaTable = GC::instance().create<SharedTable>();
+                dumpTable(metaTable, luaMetatable, visited);
+                table.setMetatable(metaTable);
+            }
             return std::make_unique<SharedTableHolder>(table.handle());
         }
         default:
@@ -228,9 +235,23 @@ StoredObject createStoredObject(const char* value) {
     return std::make_unique<PrimitiveHolder<std::string>>(value);
 }
 
-StoredObject createStoredObject(const sol::object& object) { return fromSolObject(object); }
+StoredObject createStoredObject(const sol::object& object) {
+    SolTableToShared visited;
+    return fromSolObject(object, visited);
+}
 
-StoredObject createStoredObject(const sol::stack_object& object) { return fromSolObject(object); }
+StoredObject createStoredObject(const sol::stack_object& object) {
+    SolTableToShared visited;
+    return fromSolObject(object, visited);
+}
+
+StoredObject createStoredObject(const sol::object& obj, SolTableToShared& visited) {
+    return fromSolObject(obj, visited);
+}
+
+StoredObject createStoredObject(const sol::stack_object& obj, SolTableToShared& visited) {
+    return fromSolObject(obj, visited);
+}
 
 template <typename DataType>
 sol::optional<DataType> getPrimitiveHolderData(const StoredObject& sobj) {

src/cpp/stored-object.h

@@ -52,6 +52,11 @@ StoredObject createStoredObject(const char*);
 StoredObject createStoredObject(const sol::object&);
 StoredObject createStoredObject(const sol::stack_object&);
 
+using SolTableToShared = std::vector<std::pair<sol::table, GCHandle>>;
+
+StoredObject createStoredObject(const sol::object& obj, SolTableToShared& visited);
+StoredObject createStoredObject(const sol::stack_object& obj, SolTableToShared& visited);
+
 sol::optional<bool> storedObjectToBool(const StoredObject&);
 sol::optional<double> storedObjectToDouble(const StoredObject&);
 sol::optional<LUA_INDEX_TYPE> storedObjectToIndexType(const StoredObject&);

tests/lua/dump_table.lua

@@ -88,3 +88,16 @@ test.dump_table.upvalues_with_loop = function()
     result.foo()
     test.equal(result.key, "value")
 end
+
+test.dump_table.with_metatable = function()
+    local tbl = effil.setmetatable({}, effil.setmetatable({a=1}, {b = 2}))
+    local dumped = effil.dump(tbl)
+
+    local mt = getmetatable(dumped)
+    test.not_equal(mt, nil)
+    test.equal(mt.a, 1)
+
+    local mt2 = getmetatable(mt)
+    test.not_equal(mt2, nil)
+    test.equal(mt2.b, 2)
+end

tests/lua/metatable.lua

@@ -182,6 +182,52 @@ test.shared_table_with_metatable.as_shared_table = function()
     test.equal(share.table_key, "mt_table_value")
 end
 
+test.shared_table_with_metatable.metatable_serialization = function()
+    local table_with_mt = setmetatable({}, {a=1})
+    local tbl = effil.table(table_with_mt)
+
+    test.not_equal(effil.getmetatable(tbl), nil)
+    test.equal(effil.getmetatable(tbl).a, 1)
+end
+
+test.shared_table_with_metatable.metatable_with_function_with_upvalues = function()
+    local common_table = {aa=2}
+    local tbl = setmetatable({}, {
+        a = function() return common_table end,
+        b = function() return common_table end
+    })
+
+    local mt = effil.getmetatable(effil.table(tbl))
+    test.equal(
+        select(2, debug.getupvalue(mt.a, 1)),
+        select(2, debug.getupvalue(mt.b, 1))
+    )
+end
+
+test.shared_table_with_metatable.check_eq_metamethod = function()
+    local left_table = effil.table()
+    local right_table = effil.table()
+    local left_table_clone = (effil.table{ left_table }[1]) -- userdata will change
+
+    test.is_false(left_table == right_table)
+    test.is_true(left_table == left_table_clone)
+    test.is_false(left_table == effil.channel())
+
+    effil.setmetatable(left_table, { __eq = function() return false end })
+    test.is_false(left_table == left_table_clone)
+
+    effil.setmetatable(left_table, { __eq = function() return true end })
+    test.is_true(left_table == right_table)
+    test.is_false(left_table == effil.channel())
+
+    effil.setmetatable(left_table, { __eq = function() return false end })
+    effil.setmetatable(right_table, { __eq = function() return true end })
+    test.is_false(left_table == right_table)
+
+    effil.setmetatable(left_table, nil)
+    test.is_true(left_table == right_table)
+end
+
 test.shared_table_with_metatable.table_as_index = function()
     local tbl = effil.table{}
     local mt = effil.table{ a = 1 }

tests/lua/type_mismatch.lua

@@ -88,7 +88,7 @@ local function generate_tests()
         -- effil.setmetatable
         if typename ~= "table" and typename ~= "effil.table" then
             test.type_mismatch.input_types_mismatch_p(1, "table", "setmetatable", type_instance, 44)
-            test.type_mismatch.input_types_mismatch_p(2, "table", "setmetatable", {}, type_instance)
+            test.type_mismatch.input_types_mismatch_p(2, "table or nil", "setmetatable", {}, type_instance)
         end
 
         if typename ~= "effil.table" then