New API function 'lua_closeslot'

Closing a to-be-closed variable with 'lua_settop' is too restrictive,
as it erases all slots above the variable. Moreover, it adds side
effects to 'lua_settop', which should be a fairly basic function.

6 files changed, 66 insertions, 29 deletions

diff --git a/lapi.c b/lapi.c
index 00e95a11..0f0e31af 100644
--- a/lapi.c
+++ b/lapi.c
@@ -187,9 +187,26 @@ LUA_API void lua_settop (lua_State *L, int idx) {
     api_check(L, -(idx+1) <= (L->top - (func + 1)), "invalid new top");
     diff = idx + 1;  /* will "subtract" index (as it is negative) */
   }
+#if defined(LUA_COMPAT_5_4_0)
   if (diff < 0 && hastocloseCfunc(ci->nresults))
     luaF_close(L, L->top + diff, CLOSEKTOP);
-  L->top += diff;  /* correct top only after closing any upvalue */
+#endif
+  L->top += diff;
+  api_check(L, L->openupval == NULL || uplevel(L->openupval) < L->top,
+               "cannot pop an unclosed slot");
+  lua_unlock(L);
+}
+
+
+LUA_API void lua_closeslot (lua_State *L, int idx) {
+  StkId level;
+  lua_lock(L);
+  level = index2stack(L, idx);
+  api_check(L, hastocloseCfunc(L->ci->nresults) && L->openupval != NULL &&
+               uplevel(L->openupval) == level,
+     "no variable to close at given level");
+  luaF_close(L, level, CLOSEKTOP);
+  setnilvalue(s2v(level));
   lua_unlock(L);
 }
 
diff --git a/lauxlib.c b/lauxlib.c
index 074ff08c..e8fc486e 100644
--- a/lauxlib.c
+++ b/lauxlib.c
@@ -545,10 +545,8 @@ static char *prepbuffsize (luaL_Buffer *B, size_t sz, int boxidx) {
     if (buffonstack(B))  /* buffer already has a box? */
       newbuff = (char *)resizebox(L, boxidx, newsize);  /* resize it */
     else {  /* no box yet */
-      lua_pushnil(L);  /* reserve slot for final result */
       newbox(L);  /* create a new box */
-      /* move box (and slot) to its intended position */
-      lua_rotate(L, boxidx - 1, 2);
+      lua_insert(L, boxidx);  /* move box to its intended position */
       lua_toclose(L, boxidx);
       newbuff = (char *)resizebox(L, boxidx, newsize);
       memcpy(newbuff, B->b, B->n * sizeof(char));  /* copy original content */
@@ -585,8 +583,8 @@ LUALIB_API void luaL_pushresult (luaL_Buffer *B) {
   lua_State *L = B->L;
   lua_pushlstring(L, B->b, B->n);
   if (buffonstack(B)) {
-    lua_copy(L, -1, -3);  /* move string to reserved slot */
-    lua_pop(L, 2);  /* pop string and box (closing the box) */
+    lua_closeslot(L, -2);  /* close the box */
+    lua_remove(L, -2);  /* remove box from the stack */
   }
 }
 
diff --git a/ltests.c b/ltests.c
index 6920dd69..9c13338a 100644
--- a/ltests.c
+++ b/ltests.c
@@ -1766,6 +1766,9 @@ static struct X { int x; } x;
     else if EQ("toclose") {
       lua_toclose(L1, getnum);
     }
+    else if EQ("closeslot") {
+      lua_closeslot(L1, getnum);
+    }
     else luaL_error(L, "unknown instruction %s", buff);
   }
   return 0;
diff --git a/lua.h b/lua.h
index c9d64d7f..aec70dac 100644
--- a/lua.h
+++ b/lua.h
@@ -347,7 +347,8 @@ LUA_API size_t   (lua_stringtonumber) (lua_State *L, const char *s);
 LUA_API lua_Alloc (lua_getallocf) (lua_State *L, void **ud);
 LUA_API void      (lua_setallocf) (lua_State *L, lua_Alloc f, void *ud);
 
-LUA_API void  (lua_toclose) (lua_State *L, int idx);
+LUA_API void (lua_toclose) (lua_State *L, int idx);
+LUA_API void (lua_closeslot) (lua_State *L, int idx);
 
 
 /*
diff --git a/manual/manual.of b/manual/manual.of
index c5385258..09297a63 100644
--- a/manual/manual.of
+++ b/manual/manual.of
@@ -3095,6 +3095,18 @@ will probably need to close states as soon as they are not needed.
 
 }
 
+@APIEntry{void lua_closeslot (lua_State *L, int index);|
+@apii{0,0,e}
+
+Close the to-be-closed slot at the given index and set its value to @nil.
+The index must be the last index previously marked to be closed
+@see{lua_toclose} that is still active (that is, not closed yet).
+
+(Exceptionally, this function was introduced in release 5.4.3.
+It is not present in previous 5.4 releases.)
+
+}
+
 @APIEntry{int lua_compare (lua_State *L, int index1, int index2, int op);|
 @apii{0,0,e}
 
@@ -3747,9 +3759,7 @@ except that it allows the called function to yield @see{continuations}.
 @apii{n,0,e}
 
 Pops @id{n} elements from the stack.
-
-This function can run arbitrary code when removing an index
-marked as to-be-closed from the stack.
+It is implemented as a macro over @Lid{lua_settop}.
 
 }
 
@@ -4240,8 +4250,12 @@ If the new top is greater than the old one,
 then the new elements are filled with @nil.
 If @id{index} @N{is 0}, then all stack elements are removed.
 
-This function can run arbitrary code when removing an index
-marked as to-be-closed from the stack.
+For compatibility reasons,
+this function may close slots marked as to-be-closed @see{lua_toclose},
+and therefore it can run arbitrary code.
+You should not rely on this behavior:
+Instead, always close to-be-closed slots explicitly,
+with @Lid{lua_closeslot}, before removing them from the stack.
 
 }
 
@@ -4337,10 +4351,9 @@ when it goes out of scope.
 Here, in the context of a C function,
 to go out of scope means that the running function returns to Lua,
 there is an error,
-or the index is removed from the stack through
-@Lid{lua_settop} or @Lid{lua_pop}.
-An index marked as to-be-closed should not be removed from the stack
-by any other function in the API except @Lid{lua_settop} or @Lid{lua_pop}.
+or there is a call to @Lid{lua_closeslot}.
+An index marked as to-be-closed should neither be removed from the stack
+nor modified before a corresponding call to @Lid{lua_closeslot}.
 
 This function should not be called for an index
 that is equal to or below an active to-be-closed index.
@@ -4353,7 +4366,7 @@ Note that, both in case of errors and of a regular return,
 by the time the @idx{__close} metamethod runs,
 the @N{C stack} was already unwound,
 so that any automatic @N{C variable} declared in the calling function
-will be out of scope.
+(e.g., a buffer) will be out of scope.
 
 }
 
diff --git a/testes/api.lua b/testes/api.lua
index 95551481..fb7e7085 100644
--- a/testes/api.lua
+++ b/testes/api.lua
@@ -507,10 +507,12 @@ function checkerrnopro (code, msg)
 end
 
 if not _soft then
+  collectgarbage("stop")   -- avoid __gc with full stack
   checkerrnopro("pushnum 3; call 0 0", "attempt to call")
   print"testing stack overflow in unprotected thread"
   function f () f() end
   checkerrnopro("getglobal 'f'; call 0 0;", "stack overflow")
+  collectgarbage("restart")
 end
 print"+"
 
@@ -1125,26 +1127,29 @@ do
     assert(#openresource == n)
   end
 
-  -- closing resources with 'settop'
+  -- closing resources with 'closeslot'
+  _ENV.xxx = true
   local a = T.testC([[
-    pushvalue 2
-    call 0 1   # create resource
+    pushvalue 2  # stack: S, NR, CH
+    call 0 1   # create resource; stack: S, NR, CH, R
     toclose -1 # mark it to be closed
-    pushvalue 2
-    call 0 1   # create another resource
+    pushvalue 2  #  stack: S, NR, CH, R, NR
+    call 0 1   # create another resource; stack: S, NR, CH, R, R
     toclose -1 # mark it to be closed
-    pushvalue 3
+    pushvalue 3  # stack: S, NR, CH, R, R, CH
     pushint 2   # there should be two open resources
-    call 1 0
-    pop 1       # pop second resource from the stack
-    pushvalue 3
+    call 1 0  #  stack: S, NR, CH, R, R
+    closeslot -1   # close second resource
+    pushvalue 3  # stack: S, NR, CH, R, R, CH
     pushint 1   # there should be one open resource
-    call 1 0
-    pop 1       # pop second resource from the stack
+    call 1 0  # stack: S, NR, CH, R, R
+    closeslot 4
+    setglobal "xxx"  # previous op. erased the slot
+    pop 1       # pop other resource from the stack
     pushint *
     return 1    # return stack size
   ]], newresource, check)
-  assert(a == 3)   -- no extra items left in the stack
+  assert(a == 3 and _ENV.xxx == nil)   -- no extra items left in the stack
 
   -- non-closable value
   local a, b = pcall(T.makeCfunc[[