From cffeb0d4debed48c5ec7d0ec9cce993260b8b35f Mon Sep 17 00:00:00 2001 From: Michal Kottman Date: Wed, 19 Dec 2012 12:53:37 +0100 Subject: [PATCH] First attempt at safe virtual overrides Virtual methods implemented in Lua are not called until a flag is set in the bitset hasOverride in the shell class. This bitfield is updated in the __newindex by checking if the value is a function and key is a name known at compile time. The overhead at __newindex is lower lower than actually checking for overrides at runtime, and also possibly safer - no Lua calls are made (apart from lua_gettop) when the override is not present. --- common/lqt_common.cpp | 49 +++++++++++++++++++++++++++++++++++++++- common/lqt_common.hpp | 2 +- generator/classes.lua | 26 ++++++++++++++++++++- generator/qt_internal.lua | 6 +++++ generator/virtuals.lua | 57 ++++++++++++++++++++++++++++++++++++----------- test/virt_test.lua | 10 +++++++-- 6 files changed, 132 insertions(+), 18 deletions(-) diff --git a/common/lqt_common.cpp b/common/lqt_common.cpp index 56c7a7c..c28e114 100644 --- a/common/lqt_common.cpp +++ b/common/lqt_common.cpp @@ -194,8 +194,37 @@ static int lqtL_gcfunc (lua_State *L) { return 0; // (+0) } +static void dumpStack(const char *msg, lua_State* l) { + int i; + int top = lua_gettop(l); + + printf("=== %s: %d {\n", msg, top); + + for (i = 1; i <= top; i++) + { /* repeat for each level */ + int t = lua_type(l, i); + switch (t) { + case LUA_TSTRING: /* strings */ + printf("string: '%s'\n", lua_tostring(l, i)); + break; + case LUA_TBOOLEAN: /* booleans */ + printf("boolean %s\n",lua_toboolean(l, i) ? "true" : "false"); + break; + case LUA_TNUMBER: /* numbers */ + printf("number: %g\n", lua_tonumber(l, i)); + break; + default: /* other values */ + printf("%s: %p\n", lua_typename(l, t), lua_topointer(l, i)); + break; + } + } + printf("} ===\n"); /* end the listing */ +} + static int lqtL_newindexfunc (lua_State *L) { if (!lua_isuserdata(L, 1) && lua_islightuserdata(L, 1)) return 0; + + // first try a setter lua_getmetatable(L, 1); lua_pushliteral(L, "__set"); lua_rawget(L, -2); @@ -208,6 +237,17 @@ static int lqtL_newindexfunc (lua_State *L) { return setter(L); } } + + // then try marking a method override + lua_settop(L, 4); + lua_pushliteral(L, "__override"); + lua_rawget(L, -2); + if (lua_iscfunction(L, -1) && lua_isfunction(L, 3)) { + lua_CFunction addOverride = lua_tocfunction(L, -1); + addOverride(L); + } + + // anyway, use the environment table for the userdata as per-object storage lua_settop(L, 3); // (=3) lua_getfenv(L, 1); // (+1) if (!lua_istable(L, -1)) { @@ -293,7 +333,10 @@ static int lqtL_local_ctor(lua_State*L) { return lua_gettop(L); } -int lqtL_createclass (lua_State *L, const char *name, luaL_Reg *mt, luaL_Reg *getters, luaL_Reg *setters, lqt_Base *bases) { +int lqtL_createclass (lua_State *L, const char *name, luaL_Reg *mt, + luaL_Reg *getters, luaL_Reg *setters, lua_CFunction override, + lqt_Base *bases) +{ int len = 0; char *new_name = NULL; lqt_Base *bi = bases; @@ -320,6 +363,10 @@ int lqtL_createclass (lua_State *L, const char *name, luaL_Reg *mt, luaL_Reg *ge luaL_register(L, NULL, setters); lua_setfield(L, -2, "__set"); } + if (override) { + lua_pushcfunction(L, override); + lua_setfield(L, -2, "__override"); + } // set metafunctions lqtL_pushindexfunc(L, name, bases); // (2) diff --git a/common/lqt_common.hpp b/common/lqt_common.hpp index de53760..db6ffe0 100644 --- a/common/lqt_common.hpp +++ b/common/lqt_common.hpp @@ -137,7 +137,7 @@ typedef struct { const char * name; } lqt_Class; -int lqtL_createclass (lua_State *, const char *, luaL_Reg *, luaL_Reg *, luaL_Reg *, lqt_Base *); +int lqtL_createclass (lua_State *, const char *, luaL_Reg *, luaL_Reg *, luaL_Reg *, lua_CFunction, lqt_Base *); /* functions to get/push special types */ diff --git a/generator/classes.lua b/generator/classes.lua index 79de147..fb67533 100644 --- a/generator/classes.lua +++ b/generator/classes.lua @@ -764,10 +764,17 @@ function print_single_class(c) getters_setters = 'lqt_getters'..c.xarg.id..', lqt_setters'..c.xarg.id end + local shellname = 'lqt_shell_'..n + + local overrides = 'NULL' + if c.shell then + overrides = shellname..'::lqtAddOverride' + end + print_meta('extern "C" LQT_EXPORT int luaopen_'..n..' (lua_State *L) {') print_meta('\tlqtL_createclass(L, "' ..lua_name..'*", lqt_metatable' - ..c.xarg.id..', '..getters_setters..', lqt_base' + ..c.xarg.id..', '..getters_setters..', '..overrides..', lqt_base' ..c.xarg.id..');') if c.implicit then @@ -784,6 +791,23 @@ function print_single_class(c) print_meta'\treturn 0;' print_meta'}' print_meta'' + + if c.shell then + print_meta('int '..shellname..'::lqtAddOverride(lua_State *L) {') + print_meta(' '..shellname..' *self = static_cast<'..shellname..'*>('..typesystem[c.xarg.fullname..'*'].get(1)..');') + print_meta(' const char *name = luaL_checkstring(L, 2);') + print_meta(' // printf("Overriding %s in %s\\n", name, "'..shellname..'");') + local virt = virtuals.sort_by_index(c) + for _, v in pairs(virt) do + print_meta(' if (!strcmp(name, "'..v.xarg.name..'")) {') + print_meta(' self->hasOverride.setBit('..v.virtual_index..');') + print_meta(' // printf("-> updated %d to %d\\n", '..v.virtual_index..', (bool)self->hasOverride['..v.virtual_index..']);') + print_meta(' return 0;') + print_meta(' }') + end + print_meta('}\n\n') + end + if c.shell and c.qobject then print_meta([[ #include diff --git a/generator/qt_internal.lua b/generator/qt_internal.lua index 5a46dc8..99f3da3 100644 --- a/generator/qt_internal.lua +++ b/generator/qt_internal.lua @@ -56,6 +56,12 @@ for c in pairs(classes) do or c.xarg.fullname=='QFutureWatcherBase' -- const virtual method causes it to be abstract or c.xarg.fullname=='QEasingCurve' -- wrapper for function: function pointer parsing problem or c.xarg.fullname=='QHashData' -- not in the docs at all. free_helper is not present during compilation + + or c.xarg.fullname=='QWebSelectMethod' + or c.xarg.fullname=='QWebNotificationPresenter' + or c.xarg.fullname=='QWebHapticFeedbackPlayer' + or c.xarg.fullname=='QWebTouchModifier' + or string.match(c.xarg.fullname, '^QtConcurrent') -- does not make sense anyway, because we should duplicate the lua_State or string.match(c.xarg.fullname, '^QAccessible') -- causes a lot of headaches, and not necessarry anyway (yet) or string.match(c.xarg.fullname, 'Private$') -- should not bind these diff --git a/generator/virtuals.lua b/generator/virtuals.lua index ffdeba2..f424fd4 100644 --- a/generator/virtuals.lua +++ b/generator/virtuals.lua @@ -9,34 +9,49 @@ function fill_virtuals(classes) end local function get_virtuals(c, includePrivate) local ret = {} + local virtual_index = 0 + local function add_overload(name, func) + virtual_index = virtual_index + 1 + func.virtual_index = virtual_index + ret[name] = func + end + + -- add virtual methods declared in the class for _, f in ipairs(c) do if f.label=='Function' and f.xarg.virtual=='1' then local n = string.match(f.xarg.name, '~') or f.xarg.name - if n~='~' and n~='metaObject' then ret[n] = f end + if n~='~' and n~='metaObject' then + add_overload(n, f) + end end end + + -- find virtual methods in base classes for b in string.gmatch(c.xarg.bases or '', '([^;]+);') do local base = byname[b] if type(base)=='table' then local bv = get_virtuals(base, true) for n, f in pairs(bv) do - if not ret[n] then ret[n] = f end + if not ret[n] then add_overload(n, f) end end end end + + -- mark methods in class not explicitly marked as virtual, as C++ + -- does not require that overriden methods are marked virtual for _, f in ipairs(c) do + local n = string.match(f.xarg.name, '~') or f.xarg.name if f.label=='Function' and (includePrivate or f.xarg.access~='private') - and (ret[string.match(f.xarg.name, '~') or f.xarg.name]) then + and (ret[n]) + then f.xarg.virtual = '1' - local n = string.match(f.xarg.name, '~')or f.xarg.name - ret[n] = f end end - return ret + return ret, virtual_index end for c in pairs(classes) do - c.virtuals = get_virtuals(c) + c.virtuals, c.nvirtuals = get_virtuals(c) end end @@ -83,12 +98,19 @@ function virtual_overload(v) fallback = fallback .. (i>1 and ', arg' or 'arg') .. i end proto = proto .. ')' .. (v.xarg.constant=='1' and ' const' or '') - fallback = (v.return_type and 'return this->' or 'this->') .. v.xarg.fullname .. '(' .. fallback .. ');' + fallback = (v.return_type and 'return this->' or 'this->') .. v.xarg.fullname .. '(' .. fallback .. ');' .. + (v.return_type and '' or ' return;') if v.xarg.abstract then fallback = 'luaL_error(L, "Abstract method %s not implemented! In %s", "' .. v.xarg.name .. '", lqtL_source(L,oldtop+1));' end - ret = proto .. [[ { - int oldtop = lua_gettop(L); + ret = proto .. ' {\n' + ret = ret .. ' int oldtop = lua_gettop(L);\n' + ret = ret .. ' printf("Checking for override idx: %d name: %s class: %s value: %d in thread: %lx\\n", ' .. + v.virtual_index .. ', "'..v.xarg.name..'", "'.. v.xarg.member_of_class.. '", ' .. + '(int)(bool)hasOverride[' .. v.virtual_index .. '], QThread::currentThreadId());\n' + ret = ret .. ' if (!hasOverride[' .. v.virtual_index .. ']) { \n' + ret = ret .. ' ' .. fallback .. '\n }\n' + ret = ret .. [[ lqtL_pushudata(L, this, "]]..string.gsub(v.xarg.member_of_class, '::', '.')..[[*"); lqtL_getoverload(L, -1, "]]..v.xarg.name..[["); lua_pushvalue(L, -1); // copy of function @@ -138,8 +160,11 @@ end function fill_shell_class(c) local shellname = 'lqt_shell_'..c.xarg.cname - local shell = 'class LQT_EXPORT ' .. shellname .. ' : public ' .. c.xarg.fullname .. ' {\npublic:\n' + local shell = 'class LQT_EXPORT ' .. shellname .. ' : public ' .. c.xarg.fullname .. ' {\n' shell = shell .. ' lua_State *L;\n' + shell = shell .. ' QBitArray hasOverride;\n' + shell = shell .. 'public:\n' + shell = shell .. ' static int lqtAddOverride(lua_State *L);\n' for _, constr in ipairs(c.constructors) do if constr.xarg.access~='private' then local cline = ' '..shellname..' (lua_State *l' @@ -149,7 +174,7 @@ function fill_shell_class(c) argline = argline .. (i>1 and ', arg' or 'arg') .. i end cline = cline .. ') : ' .. c.xarg.fullname - .. '(' .. argline .. '), L(l) ' + .. '(' .. argline .. '), L(l), hasOverride(' .. c.nvirtuals .. ') ' .. '{\n lqtL_register(L, this);\n' if c.protected_enums then cline = cline .. ' registerEnums();\n' @@ -250,4 +275,10 @@ function print_virtual_overloads(classes) return classes end - +function sort_by_index(c) + local res = {} + for name, virt in pairs(c.virtuals) do + res[virt.virtual_index] = virt + end + return res +end \ No newline at end of file diff --git a/test/virt_test.lua b/test/virt_test.lua index 1bc391e..d1b1be1 100644 --- a/test/virt_test.lua +++ b/test/virt_test.lua @@ -5,8 +5,14 @@ require'qtcore' qa = QCoreApplication.new(1, {'virt_test'}) qo = QObject.new() -qo.event = function(o, e) print('event overload called', e:type()) end -qo.childEvent = function(o, e) print('child event overload called', e:type()) end +qo.event = function(o, e) + print('event overload called', e:type()) + return false +end +qo.childEvent = function(o, e) + print('child event overload called', e:type()) + return false + end ql = {} -- 2.11.4.GIT