Correct order of return hooks vs. close metamethods

The return hook should be called only after closing variables (which
are still part of the function). C functions were calling the hook
before the metamethods.

2 files changed, 85 insertions, 16 deletions

diff --git a/ldo.c b/ldo.c
index 9e3d6955..57d4f7d5 100644
--- a/ldo.c
+++ b/ldo.c
@@ -341,7 +341,8 @@ void luaD_hookcall (lua_State *L, CallInfo *ci) {
 }
 
 
-static StkId rethook (lua_State *L, CallInfo *ci, StkId firstres, int nres) {
+static void rethook (lua_State *L, CallInfo *ci, int nres) {
+  StkId firstres = L->top - nres;  /* index of first result */
   ptrdiff_t oldtop = savestack(L, L->top);  /* hook may change top */
   int delta = 0;
   if (isLuacode(ci)) {
@@ -360,7 +361,7 @@ static StkId rethook (lua_State *L, CallInfo *ci, StkId firstres, int nres) {
   }
   if (isLua(ci = ci->previous))
     L->oldpc = pcRel(ci->u.l.savedpc, ci_func(ci)->p);  /* update 'oldpc' */
-  return restorestack(L, oldtop);
+  L->top = restorestack(L, oldtop);
 }
 
 
@@ -397,7 +398,7 @@ static void moveresults (lua_State *L, StkId res, int nres, int wanted) {
     case 1:  /* one value needed */
       if (nres == 0)   /* no results? */
         setnilvalue(s2v(res));  /* adjust with nil */
-      else
+      else  /* at least one result */
         setobjs2s(L, res, L->top - nres);  /* move it to proper place */
       L->top = res + 1;
       return;
@@ -412,6 +413,8 @@ static void moveresults (lua_State *L, StkId res, int nres, int wanted) {
         wanted = codeNresults(wanted);  /* correct value */
         if (wanted == LUA_MULTRET)
           wanted = nres;
+        if (L->hookmask)  /* if needed, call hook after '__close's */
+          rethook(L, L->ci, nres);
       }
       break;
   }
@@ -426,15 +429,18 @@ static void moveresults (lua_State *L, StkId res, int nres, int wanted) {
 
 
 /*
-** Finishes a function call: calls hook if necessary, removes CallInfo,
-** moves current number of results to proper place.
+** Finishes a function call: calls hook if necessary, moves current
+** number of results to proper place, and returns to previous call
+** info. If function has to close variables, hook must be called after
+** that.
 */
 void luaD_poscall (lua_State *L, CallInfo *ci, int nres) {
-  if (L->hookmask)
-    L->top = rethook(L, ci, L->top - nres, nres);
-  L->ci = ci->previous;  /* back to caller */
+  int wanted = ci->nresults;
+  if (L->hookmask && !hastocloseCfunc(wanted))
+    rethook(L, ci, nres);
   /* move results to proper place */
-  moveresults(L, ci->func, nres, ci->nresults);
+  moveresults(L, ci->func, nres, wanted);
+  L->ci = ci->previous;  /* back to caller (after closing variables) */
 }
 
 
diff --git a/testes/locals.lua b/testes/locals.lua
index 24a95d18..a25b2b9f 100644
--- a/testes/locals.lua
+++ b/testes/locals.lua
@@ -521,6 +521,14 @@ do   -- tbc inside close methods
 end
 
 
+local function checktable (t1, t2)
+  assert(#t1 == #t2)
+  for i = 1, #t1 do
+    assert(t1[i] == t2[i])
+  end
+end
+
+
 if rawget(_G, "T") then
 
   -- memory error inside closing function
@@ -632,6 +640,68 @@ if rawget(_G, "T") then
     print'+'
   end
 
+
+  do
+    -- '__close' vs. return hooks in C functions
+    local trace = {}
+
+    local function hook (event)
+      trace[#trace + 1] = event .. " " .. (debug.getinfo(2).name or "?")
+    end
+
+    -- create tbc variables to be used by C function
+    local x = func2close(function (_,msg)
+      trace[#trace + 1] = "x"
+    end)
+
+    local y = func2close(function (_,msg)
+      trace[#trace + 1] = "y"
+    end)
+
+    debug.sethook(hook, "r")
+    local t = {T.testC([[
+       toclose 2      # x
+       pushnum 10
+       pushint 20
+       toclose 3      # y
+       return 2
+    ]], x, y)}
+    debug.sethook()
+
+    -- hooks ran before return hook from 'testC'
+    checktable(trace,
+       {"return sethook", "y", "return ?", "x", "return ?", "return testC"})
+    -- results are correct
+    checktable(t, {10, 20})
+  end
+
+end
+
+
+do   -- '__close' vs. return hooks in Lua functions
+  local trace = {}
+
+  local function hook (event)
+    trace[#trace + 1] = event .. " " .. debug.getinfo(2).name
+  end
+
+  local function foo (...)
+    local x <close> = func2close(function (_,msg)
+      trace[#trace + 1] = "x"
+    end)
+
+    local y <close> = func2close(function (_,msg)
+      debug.sethook(hook, "r")
+    end)
+
+    return ...
+  end
+
+  local t = {foo(10,20,30)}
+  debug.sethook()
+  checktable(t, {10, 20, 30})
+  checktable(trace,
+    {"return sethook", "return close", "x", "return close", "return foo"})
 end
 
 
@@ -640,13 +710,6 @@ print "to-be-closed variables in coroutines"
 do
   -- yielding inside closing metamethods
 
-  local function checktable (t1, t2)
-    assert(#t1 == #t2)
-    for i = 1, #t1 do
-      assert(t1[i] == t2[i])
-    end
-  end
-
   local trace = {}
   local co = coroutine.wrap(function ()