Process upvalues equal to _G in Lua51 as in other flavors

9 files changed, 59 insertions, 90 deletions

diff --git a/src/intercopycontext.cpp b/src/intercopycontext.cpp
index adbb502..6684f3f 100644
--- a/src/intercopycontext.cpp
+++ b/src/intercopycontext.cpp
@@ -209,19 +209,17 @@ void InterCopyContext::copy_func() const
     {
         char const* _fname{};
 #define LOG_FUNC_INFO 0
-#if LOG_FUNC_INFO
-        // "To get information about a function you push it onto the
-        // stack and start the what string with the character '>'."
-        //
+        if constexpr (LOG_FUNC_INFO)
         {
             lua_Debug _ar;
             lua_pushvalue(L1, L1_i);                                                               // L1: ... b f
+            // "To get information about a function you push it onto the stack and start the what string with the character '>'."
             // fills 'fname' 'namewhat' and 'linedefined', pops function
             lua_getinfo(L1, ">nS", &_ar);                                                          // L1: ... b
             _fname = _ar.namewhat;
             DEBUGSPEW_CODE(DebugSpew(U) << "FNAME: " << _ar.short_src << " @ " << _ar.linedefined << std::endl);
         }
-#endif // LOG_FUNC_INFO
+
         {
             std::string_view const _bytecode{ lua_tostringview(L1, -1) };                          // L1: ... b
             LUA_ASSERT(L1, !_bytecode.empty());
@@ -255,26 +253,20 @@ void InterCopyContext::copy_func() const
          * cache so we don't end up in eternal loop.
          * Lua5.2 and Lua5.3: one of the upvalues is _ENV, which we don't want to copy!
          * instead, the function shall have LUA_RIDX_GLOBALS taken in the destination state!
+         * TODO: this can probably be factorized as InterCopyContext::copyUpvalues(...)
          */
         int _n{ 0 };
         {
             InterCopyContext _c{ U, L2, L1, L2_cache_i, {}, VT::NORMAL, mode, {} };
-#if LUA_VERSION_NUM >= 502
-            // Starting with Lua 5.2, each Lua function gets its environment as one of its upvalues (named LUA_ENV, aka "_ENV" by default)
-            // Generally this is LUA_RIDX_GLOBALS, which we don't want to copy from the source to the destination state...
-            // -> if we encounter an upvalue equal to the global table in the source, bind it to the destination's global table
+            // if we encounter an upvalue equal to the global table in the source, bind it to the destination's global table
             lua_pushglobaltable(L1);                                                               // L1: ... _G
-#endif // LUA_VERSION_NUM
             for (_n = 0; (_c.name = lua_getupvalue(L1, L1_i, 1 + _n)) != nullptr; ++_n) {          // L1: ... _G up[n]
                 DEBUGSPEW_CODE(DebugSpew(U) << "UPNAME[" << _n << "]: " << _c.name << " -> ");
-#if LUA_VERSION_NUM >= 502
                 if (lua_rawequal(L1, -1, -2)) { // is the upvalue equal to the global table?
-                    DEBUGSPEW_CODE(DebugSpew(nullptr) << "pushing destination global scope" << std::endl);
+                    DEBUGSPEW_CODE(DebugSpew(U) << "pushing destination global scope" << std::endl);
                     lua_pushglobaltable(L2);                                                       //                                                L2: ... {cache} ... function <upvalues>
-                } else
-#endif // LUA_VERSION_NUM
-                {
-                    DEBUGSPEW_CODE(DebugSpew(nullptr) << "copying value" << std::endl);
+                } else {
+                    DEBUGSPEW_CODE(DebugSpew(U) << "copying value" << std::endl);
                     _c.L1_i = SourceIndex{ lua_gettop(L1) };
                     if (!_c.inter_copy_one()) {                                                    //                                                L2: ... {cache} ... function <upvalues>
                         raise_luaL_error(getErrL(), "Cannot copy upvalue type '%s'", luaL_typename(L1, -1));
@@ -282,12 +274,8 @@ void InterCopyContext::copy_func() const
                 }
                 lua_pop(L1, 1);                                                                    // L1: ... _G
             }
-#if LUA_VERSION_NUM >= 502
             lua_pop(L1, 1);                                                                        // L1: ...
-#endif // LUA_VERSION_NUM
-        }
-                                                                                                   //                                                L2: ... {cache} ... function + 'n' upvalues (>=0)
-
+        }                                                                                          //                                                L2: ... {cache} ... function + 'n' upvalues (>=0)
         STACK_CHECK(L1, 0);
 
         // Set upvalues (originally set to 'nil' by 'lua_load')
diff --git a/src/lane.cpp b/src/lane.cpp
index 4b6500a..7650d6b 100644
--- a/src/lane.cpp
+++ b/src/lane.cpp
@@ -254,7 +254,6 @@ static int thread_index_number(lua_State* L_)
         if (!lua_isnil(L_, -1)) { // an error was stored                                           // L_: lane n {uv} <error>
             lua_getmetatable(L_, 1);                                                               // L_: lane n {uv} <error> {mt}
             lua_replace(L_, -3);                                                                   // L_: lane n {mt} <error>
-#if LUA_VERSION_NUM == 501
             // Note: Lua 5.1 interpreter is not prepared to show
             //       non-string errors, so we use 'tostring()' here
             //       to get meaningful output.  --AKa 22-Jan-2009
@@ -266,13 +265,14 @@ static int thread_index_number(lua_State* L_)
             // Level 3 should show the line where 'h[x]' was read
             // but this only seems to work for string messages
             // (Lua 5.1.4). No idea, why.   --AKa 22-Jan-2009
-            if (!lua_isstring(L_, -1)) {
-                kCachedTostring.pushKey(L_);                                                       // L_: lane n {mt} <error> kCachedTostring
-                lua_rawget(L_, -3);                                                                // L_: lane n {mt} <error> tostring()
-                lua_insert(L_, -2);                                                                // L_: lane n {mt} tostring() <error>
-                lua_call(L_, 1, 1); // tostring(errstring)                                         // L_: lane n {mt} "error"
+            if constexpr (LUA_VERSION_NUM == 501) {
+                if (!lua_isstring(L_, -1)) {
+                    kCachedTostring.pushKey(L_);                                                   // L_: lane n {mt} <error> kCachedTostring
+                    lua_rawget(L_, -3);                                                            // L_: lane n {mt} <error> tostring()
+                    lua_insert(L_, -2);                                                            // L_: lane n {mt} tostring() <error>
+                    lua_call(L_, 1, 1); // tostring(errstring)                                     // L_: lane n {mt} "error"
+                }
             }
-#endif // LUA_VERSION_NUM == 501
             kCachedError.pushKey(L_);                                                              // L_: lane n {mt} "error" kCachedError
             lua_rawget(L_, -3);                                                                    // L_: lane n {mt} "error" error()
             lua_replace(L_, -3);                                                                   // L_: lane n error() "error"
@@ -784,20 +784,14 @@ Lane::Lane(Universe* U_, lua_State* L_, ErrorTraceLevel errorTraceLevel_)
 {
     assert(errorTraceLevel == ErrorTraceLevel::Minimal || errorTraceLevel == ErrorTraceLevel::Basic || errorTraceLevel == ErrorTraceLevel::Extended);
     kExtendedStackTraceRegKey.setValue(L_, [yes = errorTraceLevel == ErrorTraceLevel::Extended ? 1 : 0](lua_State* L_) { lua_pushboolean(L_, yes); });
-#if HAVE_LANE_TRACKING()
     U->tracker.tracking_add(this);
-#endif // HAVE_LANE_TRACKING()
 }
 
 // #################################################################################################
 
 Lane::~Lane()
 {
-    // Clean up after a (finished) thread
-    //
-#if HAVE_LANE_TRACKING()
     std::ignore = U->tracker.tracking_remove(this);
-#endif // HAVE_LANE_TRACKING()
 }
 
 // #################################################################################################
diff --git a/src/lane.h b/src/lane.h
index 3c1ab58..1fdd955 100644
--- a/src/lane.h
+++ b/src/lane.h
@@ -110,10 +110,8 @@ class Lane
     //    is still running
     // S: cleans up after itself if non-nullptr at lane exit
 
-#if HAVE_LANE_TRACKING()
     // For tracking only
     Lane* volatile tracking_next{ nullptr };
-#endif // HAVE_LANE_TRACKING()
 
     ErrorTraceLevel const errorTraceLevel{ Basic };
 
diff --git a/src/lanes.cpp b/src/lanes.cpp
index 000668a..8033de7 100644
--- a/src/lanes.cpp
+++ b/src/lanes.cpp
@@ -505,17 +505,13 @@ LUAG_FUNC(lane_new)
 
 // ################################################################################################
 
-#if HAVE_LANE_TRACKING()
-//---
 // threads() -> {}|nil
-//
 // Return a list of all known lanes
 LUAG_FUNC(threads)
 {
     LaneTracker const& _tracker = universe_get(L_)->tracker;
     return _tracker.pushThreadsTable(L_);
 }
-#endif // HAVE_LANE_TRACKING()
 
 // #################################################################################################
 // ######################################## Timer support ##########################################
@@ -652,13 +648,14 @@ LUAG_FUNC(configure)
         std::ignore = luaG_getfield(L_, 1, "demote_full_userdata");                                // L_: settings demote_full_userdata
         _U->demoteFullUserdata = lua_toboolean(L_, -1) ? true : false;
         lua_pop(L_, 1);                                                                            // L_: settings
-#if HAVE_LANE_TRACKING()
+
+        // tracking
         std::ignore = luaG_getfield(L_, 1, "track_lanes");                                         // L_: settings track_lanes
         if (lua_toboolean(L_, -1)) {
             _U->tracker.activate();
         }
         lua_pop(L_, 1);                                                                            // L_: settings
-#endif // HAVE_LANE_TRACKING()
+
         // Linked chains handling
         _U->selfdestructFirst = SELFDESTRUCT_END;
         _U->initializeAllocatorFunction(L_);
@@ -690,13 +687,13 @@ LUAG_FUNC(configure)
     lua_setfield(L_, -2, "configure");                                                             // L_: settings M
     // add functions to the module's table
     luaG_registerlibfuncs(L_, global::sLanesFunctions);
-#if HAVE_LANE_TRACKING()
+
     // register core.threads() only if settings say it should be available
     if (_U->tracker.isActive()) {
         lua_pushcfunction(L_, LG_threads);                                                         // L_: settings M LG_threads()
         lua_setfield(L_, -2, "threads");                                                           // L_: settings M
     }
-#endif // HAVE_LANE_TRACKING()
+
     STACK_CHECK(L_, 2);
 
     {
diff --git a/src/state.cpp b/src/state.cpp
index 7ce8db0..73c94f0 100644
--- a/src/state.cpp
+++ b/src/state.cpp
@@ -238,24 +238,28 @@ void InitializeOnStateCreate(Universe* U_, lua_State* L_)
 
 lua_State* create_state([[maybe_unused]] Universe* U_, lua_State* from_)
 {
-    lua_State* _L;
-#if LUAJIT_FLAVOR() == 64
-    // for some reason, LuaJIT 64 bits does not support creating a state with lua_newstate...
-    _L = luaL_newstate();
-#else  // LUAJIT_FLAVOR() == 64
-    if (U_->provideAllocator != nullptr) { // we have a function we can call to obtain an allocator
-        lua_pushcclosure(from_, U_->provideAllocator, 0);
-        lua_call(from_, 0, 1);
-        {
-            AllocatorDefinition* const def{ lua_tofulluserdata<AllocatorDefinition>(from_, -1) };
-            _L = lua_newstate(def->allocF, def->allocUD);
-        }
-        lua_pop(from_, 1);
-    } else {
-        // reuse the allocator provided when the master state was created
-        _L = lua_newstate(U_->protectedAllocator.allocF, U_->protectedAllocator.allocUD);
-    }
-#endif // LUAJIT_FLAVOR() == 64
+    lua_State* const _L {
+        std::invoke(
+            [U = U_, from = from_]() {
+                if constexpr (LUAJIT_FLAVOR() == 64) {
+                    // for some reason, LuaJIT 64 bits does not support creating a state with lua_newstate...
+                    return luaL_newstate();
+                } else {
+                    if (U->provideAllocator != nullptr) { // we have a function we can call to obtain an allocator
+                        lua_pushcclosure(from, U->provideAllocator, 0);
+                        lua_call(from, 0, 1);
+                        AllocatorDefinition* const _def{ lua_tofulluserdata<AllocatorDefinition>(from, -1) };
+                        lua_State* const _L{ lua_newstate(_def->allocF, _def->allocUD) };
+                        lua_pop(from, 1);
+                        return _L;
+                    } else {
+                        // reuse the allocator provided when the master state was created
+                        return lua_newstate(U->protectedAllocator.allocF, U->protectedAllocator.allocUD);
+                    }
+                }
+            }
+        )
+    };
 
     if (_L == nullptr) {
         raise_luaL_error(from_, "luaG_newstate() failed while creating state; out of memory");
@@ -355,20 +359,20 @@ lua_State* luaG_newstate(Universe* U_, SourceState from_, char const* libs_)
             open1lib(_L, kLanesCoreLibName);
             libs_ = nullptr; // done with libs
         } else {
-#if LUAJIT_FLAVOR() != 0 // building against LuaJIT headers, always open jit
-            DEBUGSPEW_CODE(DebugSpew(U_) << "opening 'jit' library" << std::endl);
-            open1lib(_L, LUA_JITLIBNAME);
-#endif // LUAJIT_FLAVOR()
+            if constexpr (LUAJIT_FLAVOR() != 0) { // building against LuaJIT headers, always open jit
+                DEBUGSPEW_CODE(DebugSpew(U_) << "opening 'jit' library" << std::endl);
+                open1lib(_L, LUA_JITLIBNAME);
+            }
             DEBUGSPEW_CODE(DebugSpew(U_) << "opening 'base' library" << std::endl);
-#if LUA_VERSION_NUM >= 502
-            // open base library the same way as in luaL_openlibs()
-            luaL_requiref(_L, LUA_GNAME, luaopen_base, 1);
-            lua_pop(_L, 1);
-#else  // LUA_VERSION_NUM
-            lua_pushcfunction(_L, luaopen_base);
-            lua_pushstring(_L, "");
-            lua_call(_L, 1, 0);
-#endif // LUA_VERSION_NUM
+            if constexpr (LUA_VERSION_NUM >= 502) {
+                // open base library the same way as in luaL_openlibs()
+                luaL_requiref(_L, LUA_GNAME, luaopen_base, 1);
+                lua_pop(_L, 1);
+            } else {
+                lua_pushcfunction(_L, luaopen_base);
+                lua_pushstring(_L, "");
+                lua_call(_L, 1, 0);
+            }
         }
     }
     STACK_CHECK(_L, 0);
diff --git a/src/tracker.cpp b/src/tracker.cpp
index 69cd90c..d42eb35 100644
--- a/src/tracker.cpp
+++ b/src/tracker.cpp
@@ -28,8 +28,6 @@ THE SOFTWARE.
 
 // #################################################################################################
 
-#if HAVE_LANE_TRACKING()
-
 /*
  * Add the lane to tracking chain; the ones still running at the end of the
  * whole process will be cancelled.
@@ -104,5 +102,3 @@ void LaneTracker::tracking_add(Lane* lane_)
     }
     return lua_gettop(L_) - _top;                                                                  // L_: 0 or 1
 }
-
-#endif // HAVE_LANE_TRACKING()
diff --git a/src/tracker.h b/src/tracker.h
index 087598c..14926ec 100644
--- a/src/tracker.h
+++ b/src/tracker.h
@@ -2,11 +2,6 @@
 
 #include <mutex>
 
-// Do we want to activate full lane tracking feature?
-#define HAVE_LANE_TRACKING() 1
-
-#if HAVE_LANE_TRACKING()
-
 class Lane;
 struct lua_State;
 
@@ -31,5 +26,3 @@ class LaneTracker
         return trackingFirst != nullptr;
     }
 };
-
-#endif // HAVE_LANE_TRACKING()
diff --git a/src/universe.h b/src/universe.h
index 97b613e..f5b31a3 100644
--- a/src/universe.h
+++ b/src/universe.h
@@ -150,9 +150,7 @@ class Universe
     // used for timers (each lane will get a proxy to this)
     DeepPrelude* timerLinda{ nullptr };
 
-#if HAVE_LANE_TRACKING()
     LaneTracker tracker;
-#endif // HAVE_LANE_TRACKING()
 
     // Protects modifying the selfdestruct chain
     std::mutex selfdestructMutex;
diff --git a/tests/basic.lua b/tests/basic.lua
index 5a905e7..cfe6fd5 100644
--- a/tests/basic.lua
+++ b/tests/basic.lua
@@ -142,7 +142,8 @@ if st=="done" then
 end
 assert(st=="running")
 
-lane9:cancel("count", 100) -- 0 timeout, 100 instructions count hook
+-- when running under luajit, the function is JIT-ed, and the instruction count isn't hit, so we need a different hook
+lane9:cancel(jit and "line" or "count", 100) -- 0 timeout, hook triggers cancelslation when reaching the specified count
 
 local t0= os.time()
 while os.time()-t0 < 5 do
@@ -151,7 +152,7 @@ while os.time()-t0 < 5 do
     if st~="running" then break end
 end
 PRINT(" "..st)
-assert(st == "cancelled")
+assert(st == "cancelled", "st is '" .. st .. "' instead of 'cancelled'")
 
 -- cancellation of lanes waiting on a linda
 local limited = lanes.linda("limited")