From 0ec260c12ee6a37e763bc60ef587dbd891136e76 Mon Sep 17 00:00:00 2001 From: Benoit Germain <benoit.germain@ubisoft.com> Date: Mon, 20 May 2024 11:48:41 +0200 Subject: Start using string_view --- deep_test/deep_test.cpp | 2 +- docs/index.html | 2 +- src/deep.cpp | 8 ++--- src/deep.h | 3 +- src/intercopycontext.cpp | 89 ++++++++++++++++++++++++------------------------ src/intercopycontext.h | 3 ++ src/lanes.cpp | 8 ++--- src/lindafactory.cpp | 4 +-- src/lindafactory.h | 2 +- src/tools.cpp | 42 +++++++++++------------ 10 files changed, 83 insertions(+), 80 deletions(-) diff --git a/deep_test/deep_test.cpp b/deep_test/deep_test.cpp index 35a2e56..9ca88df 100644 --- a/deep_test/deep_test.cpp +++ b/deep_test/deep_test.cpp @@ -19,7 +19,7 @@ class MyDeepFactory : public DeepFactory { luaL_getmetatable(L_, "deep"); } - [[nodiscard]] char const* moduleName() const override { return "deep_test"; } + [[nodiscard]] std::string_view moduleName() const override { return std::string_view{ "deep_test" }; } }; /*static*/ MyDeepFactory MyDeepFactory::Instance{}; diff --git a/docs/index.html b/docs/index.html index f34b672..c9c070c 100644 --- a/docs/index.html +++ b/docs/index.html @@ -1732,7 +1732,7 @@ class MyDeepFactory : public DeepFactory DeepPrelude* newDeepObjectInternal(lua_State* L) const override; void deleteDeepObjectInternal(lua_State* L, DeepPrelude* o_) const override; void createMetatable(lua_State* L) const override; - char const* moduleName() const override; + std::string_view moduleName() const override; }; static MyDeepFactory g_MyDeepFactory; diff --git a/src/deep.cpp b/src/deep.cpp index 2f4f1ed..5515a62 100644 --- a/src/deep.cpp +++ b/src/deep.cpp @@ -244,12 +244,12 @@ char const* DeepFactory::PushDeepProxy(DestState L_, DeepPrelude* prelude_, int factory.storeDeepLookup(L_); // 2 - cause the target state to require the module that exported the factory - if (char const* const _modname{ factory.moduleName() }; _modname) { // we actually got a module name + if (std::string_view const _modname{ factory.moduleName() }; !_modname.empty()) { // we actually got a module name // L.registry._LOADED exists without having registered the 'package' library. - lua_getglobal(L_, "require"); // DPC proxy metatable require() + lua_getglobal(L_, "require"); // L_: DPC proxy metatable require() // check that the module is already loaded (or being loaded, we are happy either way) if (lua_isfunction(L_, -1)) { - lua_pushstring(L_, _modname); // L_: DPC proxy metatable require() "module" + lua_pushlstring(L_, _modname.data(), _modname.size()); // L_: DPC proxy metatable require() "module" lua_getfield(L_, LUA_REGISTRYINDEX, LUA_LOADED_TABLE); // L_: DPC proxy metatable require() "module" _R._LOADED if (lua_istable(L_, -1)) { lua_pushvalue(L_, -2); // L_: DPC proxy metatable require() "module" _R._LOADED "module" @@ -262,7 +262,7 @@ char const* DeepFactory::PushDeepProxy(DestState L_, DeepPrelude* prelude_, int require_result = lua_pcall(L_, 1, 0, 0); // L_: DPC proxy metatable error? if (require_result != LUA_OK) { // failed, return the error message - lua_pushfstring(L_, "error while requiring '%s' identified by DeepFactory::moduleName: ", _modname); + lua_pushfstring(L_, "error while requiring '%s' identified by DeepFactory::moduleName: ", _modname.data()); lua_insert(L_, -2); // L_: DPC proxy metatable prefix error lua_concat(L_, 2); // L_: DPC proxy metatable error return lua_tostring(L_, -1); diff --git a/src/deep.h b/src/deep.h index 87e5329..96461d6 100644 --- a/src/deep.h +++ b/src/deep.h @@ -18,6 +18,7 @@ extern "C" #include "uniquekey.h" #include <atomic> +#include <string_view> // forwards enum class LookupMode; @@ -67,7 +68,7 @@ class DeepFactory [[nodiscard]] virtual DeepPrelude* newDeepObjectInternal(lua_State* L_) const = 0; virtual void deleteDeepObjectInternal(lua_State* L_, DeepPrelude* o_) const = 0; virtual void createMetatable(lua_State* L_) const = 0; - [[nodiscard]] virtual char const* moduleName() const = 0; + [[nodiscard]] virtual std::string_view moduleName() const = 0; public: // NVI: public interface diff --git a/src/intercopycontext.cpp b/src/intercopycontext.cpp index e2e3d31..2f83400 100644 --- a/src/intercopycontext.cpp +++ b/src/intercopycontext.cpp @@ -73,60 +73,61 @@ THE SOFTWARE. // ################################################################################################# // retrieve the name of a function/table in the lookup database -[[nodiscard]] static char const* find_lookup_name(lua_State* L_, int i_, LookupMode mode_, char const* upName_, size_t* len_) +[[nodiscard]] std::string_view InterCopyContext::findLookupName() const { - LUA_ASSERT(L_, lua_isfunction(L_, i_) || lua_istable(L_, i_)); // L_: ... v ... - STACK_CHECK_START_REL(L_, 0); - STACK_GROW(L_, 3); // up to 3 slots are necessary on error - if (mode_ == LookupMode::FromKeeper) { - lua_CFunction const _f{ lua_tocfunction(L_, i_) }; // should *always* be one of the function sentinels + LUA_ASSERT(L1, lua_isfunction(L1, L1_i) || lua_istable(L1, L1_i)); // L1: ... v ... + STACK_CHECK_START_REL(L1, 0); + STACK_GROW(L1, 3); // up to 3 slots are necessary on error + if (mode == LookupMode::FromKeeper) { + lua_CFunction const _f{ lua_tocfunction(L1, L1_i) }; // should *always* be one of the function sentinels if (_f == func_lookup_sentinel || _f == table_lookup_sentinel || _f == userdata_clone_sentinel) { - lua_getupvalue(L_, i_, 1); // L_: ... v ... "f.q.n" + lua_getupvalue(L1, L1_i, 1); // L1: ... v ... "f.q.n" } else { // if this is not a sentinel, this is some user-created table we wanted to lookup - LUA_ASSERT(L_, nullptr == _f && lua_istable(L_, i_)); + LUA_ASSERT(L1, nullptr == _f && lua_istable(L1, L1_i)); // push anything that will convert to nullptr string - lua_pushnil(L_); // L_: ... v ... nil + lua_pushnil(L1); // L1: ... v ... nil } } else { // fetch the name from the source state's lookup table - kLookupRegKey.pushValue(L_); // L_: ... v ... {} - STACK_CHECK(L_, 1); - LUA_ASSERT(L_, lua_istable(L_, -1)); - lua_pushvalue(L_, i_); // L_: ... v ... {} v - lua_rawget(L_, -2); // L_: ... v ... {} "f.q.n" + kLookupRegKey.pushValue(L1); // L1: ... v ... {} + STACK_CHECK(L1, 1); + LUA_ASSERT(L1, lua_istable(L1, -1)); + lua_pushvalue(L1, L1_i); // L1: ... v ... {} v + lua_rawget(L1, -2); // L1: ... v ... {} "f.q.n" } - char const* _fqn{ lua_tolstring(L_, -1, len_) }; - DEBUGSPEW_CODE(Universe* const _U = universe_get(L_)); + size_t _len{ 0 }; + char const* _fqn{ lua_tolstring(L1, -1, &_len) }; + DEBUGSPEW_CODE(Universe* const _U = universe_get(L1)); DEBUGSPEW_CODE(fprintf(stderr, INDENT_BEGIN "function [C] %s \n" INDENT_END(_U), _fqn)); // popping doesn't invalidate the pointer since this is an interned string gotten from the lookup database - lua_pop(L_, (mode_ == LookupMode::FromKeeper) ? 1 : 2); // L_: ... v ... - STACK_CHECK(L_, 0); - if (nullptr == _fqn && !lua_istable(L_, i_)) { // raise an error if we try to send an unknown function (but not for tables) - *len_ = 0; // just in case + lua_pop(L1, (mode == LookupMode::FromKeeper) ? 1 : 2); // L1: ... v ... + STACK_CHECK(L1, 0); + if (nullptr == _fqn && !lua_istable(L1, L1_i)) { // raise an error if we try to send an unknown function (but not for tables) + _len = 0; // just in case // try to discover the name of the function we want to send - lua_getglobal(L_, "decoda_name"); // L_: ... v ... decoda_name - char const* from{ lua_tostring(L_, -1) }; - lua_pushcfunction(L_, luaG_nameof); // L_: ... v ... decoda_name luaG_nameof - lua_pushvalue(L_, i_); // L_: ... v ... decoda_name luaG_nameof t - lua_call(L_, 1, 2); // L_: ... v ... decoda_name "type" "name"|nil - char const* typewhat{ (lua_type(L_, -2) == LUA_TSTRING) ? lua_tostring(L_, -2) : luaL_typename(L_, -2) }; + lua_getglobal(L1, "decoda_name"); // L1: ... v ... decoda_name + char const* from{ lua_tostring(L1, -1) }; + lua_pushcfunction(L1, luaG_nameof); // L1: ... v ... decoda_name luaG_nameof + lua_pushvalue(L1, L1_i); // L1: ... v ... decoda_name luaG_nameof t + lua_call(L1, 1, 2); // L1: ... v ... decoda_name "type" "name"|nil + char const* typewhat{ (lua_type(L1, -2) == LUA_TSTRING) ? lua_tostring(L1, -2) : luaL_typename(L1, -2) }; // second return value can be nil if the table was not found // probable reason: the function was removed from the source Lua state before Lanes was required. char const *what, *gotchaA, *gotchaB; - if (lua_isnil(L_, -1)) { + if (lua_isnil(L1, -1)) { gotchaA = " referenced by"; gotchaB = "\n(did you remove it from the source Lua state before requiring Lanes?)"; - what = upName_; + what = name; } else { gotchaA = ""; gotchaB = ""; - what = (lua_type(L_, -1) == LUA_TSTRING) ? lua_tostring(L_, -1) : luaL_typename(L_, -1); + what = (lua_type(L1, -1) == LUA_TSTRING) ? lua_tostring(L1, -1) : luaL_typename(L1, -1); } - raise_luaL_error(L_, "%s%s '%s' not found in %s origin transfer database.%s", typewhat, gotchaA, what, from ? from : "main", gotchaB); + raise_luaL_error(L1, "%s%s '%s' not found in %s origin transfer database.%s", typewhat, gotchaA, what, from ? from : "main", gotchaB); } - STACK_CHECK(L_, 0); - return _fqn; + STACK_CHECK(L1, 0); + return std::string_view{ _fqn, _len }; } // ################################################################################################# @@ -312,8 +313,7 @@ void InterCopyContext::copy_func() const void InterCopyContext::lookup_native_func() const { // get the name of the function we want to send - size_t _len; - char const* const _fqn{ find_lookup_name(L1, L1_i, mode, name, &_len) }; + std::string_view const _fqn{ findLookupName() }; // push the equivalent function in the destination's stack, retrieved from the lookup table STACK_CHECK_START_REL(L2, 0); STACK_GROW(L2, 3); // up to 3 slots are necessary on error @@ -324,7 +324,7 @@ void InterCopyContext::lookup_native_func() const case LookupMode::ToKeeper: // push a sentinel closure that holds the lookup name as upvalue - lua_pushlstring(L2, _fqn, _len); // L1: ... f ... L2: "f.q.n" + lua_pushlstring(L2, _fqn.data(), _fqn.size()); // L1: ... f ... L2: "f.q.n" lua_pushcclosure(L2, func_lookup_sentinel, 1); // L1: ... f ... L2: f break; @@ -333,7 +333,7 @@ void InterCopyContext::lookup_native_func() const kLookupRegKey.pushValue(L2); // L1: ... f ... L2: {} STACK_CHECK(L2, 1); LUA_ASSERT(L1, lua_istable(L2, -1)); - lua_pushlstring(L2, _fqn, _len); // L1: ... f ... L2: {} "f.q.n" + lua_pushlstring(L2, _fqn.data(), _fqn.size()); // L1: ... f ... L2: {} "f.q.n" lua_rawget(L2, -2); // L1: ... f ... L2: {} f // nil means we don't know how to transfer stuff: user should do something // anything other than function or table should not happen! @@ -413,7 +413,7 @@ void InterCopyContext::copy_cached_func() const // pushes a copy of the func, stores a reference in the cache copy_func(); // L2: ... {cache} ... function } else { // found function in the cache - lua_remove(L2, -2); // L2: ... {cache} ... function + lua_remove(L2, -2); // L2: ... {cache} ... function } STACK_CHECK(L2, 1); LUA_ASSERT(L1, lua_isfunction(L2, -1)); @@ -430,9 +430,8 @@ void InterCopyContext::copy_cached_func() const [[nodiscard]] bool InterCopyContext::lookup_table() const { // get the name of the table we want to send - size_t _len; - char const* const _fqn{ find_lookup_name(L1, L1_i, mode, name, &_len) }; - if (nullptr == _fqn) { // name not found, it is some user-created table + std::string_view const _fqn{ findLookupName() }; + if (_fqn.empty()) { // name not found, it is some user-created table return false; } // push the equivalent table in the destination's stack, retrieved from the lookup table @@ -445,7 +444,7 @@ void InterCopyContext::copy_cached_func() const case LookupMode::ToKeeper: // push a sentinel closure that holds the lookup name as upvalue - lua_pushlstring(L2, _fqn, _len); // L1: ... t ... L2: "f.q.n" + lua_pushlstring(L2, _fqn.data(), _fqn.size()); // L1: ... t ... L2: "f.q.n" lua_pushcclosure(L2, table_lookup_sentinel, 1); // L1: ... t ... L2: f break; @@ -454,12 +453,12 @@ void InterCopyContext::copy_cached_func() const kLookupRegKey.pushValue(L2); // L1: ... t ... L2: {} STACK_CHECK(L2, 1); LUA_ASSERT(L1, lua_istable(L2, -1)); - lua_pushlstring(L2, _fqn, _len); // L2: {} "f.q.n" - lua_rawget(L2, -2); // L2: {} t + lua_pushlstring(L2, _fqn.data(), _fqn.size()); // L2: {} "f.q.n" + lua_rawget(L2, -2); // L2: {} t // we accept destination lookup failures in the case of transfering the Lanes body function (this will result in the source table being cloned instead) // but not when we extract something out of a keeper, as there is nothing to clone! if (lua_isnil(L2, -1) && mode == LookupMode::LaneBody) { - lua_pop(L2, 2); // L1: ... t ... L2: + lua_pop(L2, 2); // L1: ... t ... L2: STACK_CHECK(L2, 0); return false; } else if (!lua_istable(L2, -1)) { // this can happen if someone decides to replace same already registered item (for a example a standard lib function) with a table @@ -932,7 +931,7 @@ void InterCopyContext::inter_copy_keyvaluepair() const [[nodiscard]] bool InterCopyContext::inter_copy_string() const { - size_t _len; + size_t _len{ 0 }; char const* const _s{ lua_tolstring(L1, L1_i, &_len) }; DEBUGSPEW_CODE(fprintf(stderr, "'%s'\n", _s)); lua_pushlstring(L2, _s, _len); diff --git a/src/intercopycontext.h b/src/intercopycontext.h index 28e1ead..4f6ed89 100644 --- a/src/intercopycontext.h +++ b/src/intercopycontext.h @@ -2,6 +2,8 @@ #include "tools.h" +#include <string_view> + // forwards class Universe; @@ -41,6 +43,7 @@ class InterCopyContext // when mode == LookupMode::FromKeeper, L1 is a keeper state and L2 is not, therefore L2 is the state where we want to raise the error // whon mode != LookupMode::FromKeeper, L1 is not a keeper state, therefore L1 is the state where we want to raise the error lua_State* getErrL() const { return (mode == LookupMode::FromKeeper) ? L2 : L1; } + [[nodiscard]] std::string_view findLookupName() const; // for use in copy_cached_func void copy_func() const; diff --git a/src/lanes.cpp b/src/lanes.cpp index afcac1d..aa614b7 100644 --- a/src/lanes.cpp +++ b/src/lanes.cpp @@ -395,8 +395,8 @@ LUAG_FUNC(lane_new) raise_luaL_error(L_, "required module list should be a list of strings"); } else { // require the module in the target state, and populate the lookup table there too - size_t len; - char const* name = lua_tolstring(L_, -1, &len); + size_t _len{ 0 }; + char const* _name{ lua_tolstring(L_, -1, &_len) }; DEBUGSPEW_CODE(fprintf(stderr, INDENT_BEGIN "lane_new: require '%s'\n" INDENT_END(_U), name)); // require the module in the target lane @@ -405,7 +405,7 @@ LUAG_FUNC(lane_new) lua_pop(_L2, 1); // L_: [fixed] args... n "modname" L2: raise_luaL_error(L_, "cannot pre-require modules without loading 'package' library first"); } else { - lua_pushlstring(_L2, name, len); // L_: [fixed] args... n "modname" L2: require() name + lua_pushlstring(_L2, _name, _len); // L_: [fixed] args... n "modname" L2: require() name if (lua_pcall(_L2, 1, 1, 0) != LUA_OK) { // L_: [fixed] args... n "modname" L2: ret/errcode // propagate error to main state if any InterCopyContext _c{ _U, DestState{ L_ }, SourceState{ _L2 }, {}, {}, {}, {}, {} }; @@ -414,7 +414,7 @@ LUAG_FUNC(lane_new) } // here the module was successfully required // L_: [fixed] args... n "modname" L2: ret // after requiring the module, register the functions it exported in our name<->function database - populate_func_lookup_table(_L2, -1, name); + populate_func_lookup_table(_L2, -1, _name); lua_pop(_L2, 1); // L_: [fixed] args... n "modname" L2: } } diff --git a/src/lindafactory.cpp b/src/lindafactory.cpp index 16c653f..9bc56d9 100644 --- a/src/lindafactory.cpp +++ b/src/lindafactory.cpp @@ -91,12 +91,12 @@ void LindaFactory::deleteDeepObjectInternal(lua_State* L_, DeepPrelude* o_) cons // ################################################################################################# -char const* LindaFactory::moduleName() const +std::string_view LindaFactory::moduleName() const { // linda is a special case because we know lanes must be loaded from the main lua state // to be able to ever get here, so we know it will remain loaded as long a the main state is around // in other words, forever. - return nullptr; + return std::string_view{}; } // ################################################################################################# diff --git a/src/lindafactory.h b/src/lindafactory.h index 271f9a7..06eab44 100644 --- a/src/lindafactory.h +++ b/src/lindafactory.h @@ -21,6 +21,6 @@ class LindaFactory void createMetatable(lua_State* L_) const override; void deleteDeepObjectInternal(lua_State* L_, DeepPrelude* o_) const override; - [[nodiscard]] char const* moduleName() const override; + [[nodiscard]] std::string_view moduleName() const override; [[nodiscard]] DeepPrelude* newDeepObjectInternal(lua_State* L_) const override; }; diff --git a/src/tools.cpp b/src/tools.cpp index eff865c..8ddce75 100644 --- a/src/tools.cpp +++ b/src/tools.cpp @@ -44,24 +44,24 @@ static constexpr RegistryUniqueKey kLookupCacheRegKey{ 0x9BF75F84E54B691Bull }; // ################################################################################################# +static constexpr int kWriterReturnCode{ 666 }; [[nodiscard]] static int dummy_writer([[maybe_unused]] lua_State* L_, [[maybe_unused]] void const* p_, [[maybe_unused]] size_t sz_, [[maybe_unused]] void* ud_) { - return 666; + return kWriterReturnCode; } /* * differentiation between C, bytecode and JIT-fast functions * - * - * +----------+------------+----------+ - * | bytecode | C function | JIT-fast | - * +-----------------+----------+------------+----------+ - * | lua_topointer | | | | - * +-----------------+----------+------------+----------+ - * | lua_tocfunction | nullptr | | nullptr | - * +-----------------+----------+------------+----------+ - * | lua_dump | 666 | 1 | 1 | - * +-----------------+----------+------------+----------+ + * +-------------------+------------+----------+ + * | bytecode | C function | JIT-fast | + * +-----------------+-------------------+------------+----------+ + * | lua_topointer | | | | + * +-----------------+-------------------+------------+----------+ + * | lua_tocfunction | nullptr | | nullptr | + * +-----------------+-------------------+------------+----------+ + * | lua_dump | kWriterReturnCode | 1 | 1 | + * +-----------------+-------------------+------------+----------+ */ [[nodiscard]] FuncSubType luaG_getfuncsubtype(lua_State* L_, int _i) @@ -75,12 +75,12 @@ static constexpr RegistryUniqueKey kLookupCacheRegKey{ 0x9BF75F84E54B691Bull }; lua_pushvalue(L_, _i); _mustpush = 1; } - // the provided writer fails with code 666 - // therefore, anytime we get 666, this means that lua_dump() attempted a dump + // the provided writer fails with code kWriterReturnCode + // therefore, anytime we get kWriterReturnCode, this means that lua_dump() attempted a dump // all other cases mean this is either a C or LuaJIT-fast function - int const dumpres{ lua504_dump(L_, dummy_writer, nullptr, 0) }; + int const _dumpres{ lua504_dump(L_, dummy_writer, nullptr, 0) }; lua_pop(L_, _mustpush); - if (dumpres == 666) { + if (_dumpres == kWriterReturnCode) { return FuncSubType::Bytecode; } } @@ -438,13 +438,13 @@ void populate_func_lookup_table(lua_State* L_, int i_, char const* name_) lua_rawseti(L_, kFQN, depth_); // L_: o "r" {c} {fqn} ... {?} k U {mt} --depth_; } - lua_pop(L_, 1); // L_: o "r" {c} {fqn} ... {?} k U + lua_pop(L_, 1); // L_: o "r" {c} {fqn} ... {?} k U } STACK_CHECK(L_, 2); // search in the object's uservalues { - int uvi = 1; - while (lua_getiuservalue(L_, -1, uvi) != LUA_TNONE) { // L_: o "r" {c} {fqn} ... {?} k U {u} + int _uvi{ 1 }; + while (lua_getiuservalue(L_, -1, _uvi) != LUA_TNONE) { // L_: o "r" {c} {fqn} ... {?} k U {u} if (lua_istable(L_, -1)) { // if it is a table, look inside ++depth_; lua_pushliteral(L_, "uservalue"); // L_: o "r" {c} {fqn} ... {?} k v {u} "uservalue" @@ -455,7 +455,7 @@ void populate_func_lookup_table(lua_State* L_, int i_, char const* name_) --depth_; } lua_pop(L_, 1); // L_: o "r" {c} {fqn} ... {?} k U - ++uvi; + ++_uvi; } // when lua_getiuservalue() returned LUA_TNONE, it pushed a nil. pop it now lua_pop(L_, 1); // L_: o "r" {c} {fqn} ... {?} k U @@ -510,13 +510,13 @@ int luaG_nameof(lua_State* L_) lua_rawseti(L_, -2, 1); // L_: o nil {c} {fqn} // this is where we start the search lua_pushglobaltable(L_); // L_: o nil {c} {fqn} _G - std::ignore = DiscoverObjectNameRecur(L_, 6666, 1); + std::ignore = DiscoverObjectNameRecur(L_, std::numeric_limits<int>::max(), 1); if (lua_isnil(L_, 2)) { // try again with registry, just in case... lua_pop(L_, 1); // L_: o nil {c} {fqn} lua_pushliteral(L_, "_R"); // L_: o nil {c} {fqn} "_R" lua_rawseti(L_, -2, 1); // L_: o nil {c} {fqn} lua_pushvalue(L_, LUA_REGISTRYINDEX); // L_: o nil {c} {fqn} _R - std::ignore = DiscoverObjectNameRecur(L_, 6666, 1); + std::ignore = DiscoverObjectNameRecur(L_, std::numeric_limits<int>::max(), 1); } lua_pop(L_, 3); // L_: o "result" STACK_CHECK(L_, 1); -- cgit v1.2.3-55-g6feb