luaref: simplify handling of table callables and fix leak in vim.fn.call(table)

I AM THE TABLE

src/nvim/eval.c

@@ -6263,6 +6263,7 @@ void common_function(typval_T *argvars, typval_T *rettv,
     // function(dict.MyFunc, [arg])
     arg_pt = argvars[0].vval.v_partial;
     s = partial_name(arg_pt);
+    // TODO(bfredl): do the entire nlua_is_table_from_lua dance
   } else {
     // function('MyFunc', [arg], dict)
     s = (char_u *)tv_get_string(&argvars[0]);
@@ -7362,7 +7363,6 @@ bool callback_from_typval(Callback *const callback, typval_T *const arg)
     char_u *name = nlua_register_table_as_callable(arg);
 
     if (name != NULL) {
-      func_ref(name);
       callback->data.funcref = vim_strsave(name);
       callback->type = kCallbackFuncref;
     } else {

src/nvim/eval/funcs.c

@@ -810,6 +810,7 @@ static void f_call(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     return;
   }
 
+  bool owned = false;
   char_u      *func;
   partial_T   *partial = NULL;
   dict_T      *selfdict = NULL;
@@ -820,6 +821,7 @@ static void f_call(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     func = partial_name(partial);
   } else if (nlua_is_table_from_lua(&argvars[0])) {
     func = nlua_register_table_as_callable(&argvars[0]);
+    owned = true;
   } else {
     func = (char_u *)tv_get_string(&argvars[0]);
   }
@@ -837,6 +839,9 @@ static void f_call(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   }
 
   func_call(func, &argvars[1], partial, selfdict, rettv);
+  if (owned) {
+    func_unref(func);
+  }
 }
 
 /*

src/nvim/eval/typval.c

@@ -219,6 +219,7 @@ list_T *tv_list_alloc(const ptrdiff_t len)
   list->lv_used_next = gc_first_list;
   gc_first_list = list;
   list_log(list, NULL, (void *)(uintptr_t)len, "alloc");
+  list->lua_table_ref = LUA_NOREF;
   return list;
 }
 
@@ -302,7 +303,7 @@ void tv_list_free_list(list_T *const l)
   }
   list_log(l, NULL, NULL, "freelist");
 
-  nlua_free_typval_list(l);
+  NLUA_CLEAR_REF(l->lua_table_ref);
   xfree(l);
 }
 
@@ -1404,6 +1405,8 @@ dict_T *tv_dict_alloc(void)
   d->dv_copyID = 0;
   QUEUE_INIT(&d->watchers);
 
+  d->lua_table_ref = LUA_NOREF;
+
   return d;
 }
 
@@ -1454,7 +1457,7 @@ void tv_dict_free_dict(dict_T *const d)
     d->dv_used_next->dv_used_prev = d->dv_used_prev;
   }
 
-  nlua_free_typval_dict(d);
+  NLUA_CLEAR_REF(d->lua_table_ref);
   xfree(d);
 }
 

src/nvim/lua/converter.c

@@ -400,7 +400,6 @@ nlua_pop_typval_table_processing_end:
       case LUA_TFUNCTION: {
         LuaCFunctionState *state = xmalloc(sizeof(LuaCFunctionState));
         state->lua_callable.func_ref = nlua_ref(lstate, -1);
-        state->lua_callable.table_ref = LUA_NOREF;
 
         char_u *name = register_cfunc(
             &nlua_CFunction_func_call,
@@ -412,6 +411,7 @@ nlua_pop_typval_table_processing_end:
         break;
       }
       case LUA_TUSERDATA: {
+        // TODO(bfredl): check mt.__call and convert to function?
         nlua_pushref(lstate, nlua_nil_ref);
         bool is_nil = lua_rawequal(lstate, -2, -1);
         lua_pop(lstate, 1);

src/nvim/lua/converter.h

@@ -11,7 +11,6 @@
 
 typedef struct {
   LuaRef func_ref;
-  LuaRef table_ref;
 } LuaCallable;
 
 typedef struct {

src/nvim/lua/executor.c

@@ -931,19 +931,11 @@ void nlua_pushref(lua_State *lstate, LuaRef ref)
   lua_rawgeti(lstate, LUA_REGISTRYINDEX, ref);
 }
 
+
 /// Gets a new reference to an object stored at original_ref
 ///
 /// NOTE: It does not copy the value, it creates a new ref to the lua object.
 ///       Leaves the stack unchanged.
-LuaRef nlua_newref(lua_State *lstate, LuaRef original_ref)
-{
-  nlua_pushref(lstate, original_ref);
-  LuaRef new_ref = nlua_ref(lstate, -1);
-  lua_pop(lstate, 1);
-
-  return new_ref;
-}
-
 LuaRef api_new_luaref(LuaRef original_ref)
 {
   if (original_ref == LUA_NOREF) {
@@ -951,7 +943,10 @@ LuaRef api_new_luaref(LuaRef original_ref)
   }
 
   lua_State *const lstate = nlua_enter();
-  return nlua_newref(lstate, original_ref);
+  nlua_pushref(lstate, original_ref);
+  LuaRef new_ref = nlua_ref(lstate, -1);
+  lua_pop(lstate, 1);
+  return new_ref;
 }
 
 
@@ -1061,25 +1056,13 @@ int typval_exec_lua_callable(
     typval_T *rettv
 )
 {
-  int offset = 0;
   LuaRef cb = lua_cb.func_ref;
 
-  if (cb == LUA_NOREF) {
-    // This shouldn't happen.
-    luaL_error(lstate, "Invalid function passed to VimL");
-    return ERROR_OTHER;
-  }
-
   nlua_pushref(lstate, cb);
 
-  if (lua_cb.table_ref != LUA_NOREF) {
-    offset += 1;
-    nlua_pushref(lstate, lua_cb.table_ref);
-  }
-
   PUSH_ALL_TYPVALS(lstate, argvars, argcount, false);
 
-  if (lua_pcall(lstate, argcount + offset, 1, 0)) {
+  if (lua_pcall(lstate, argcount, 1, 0)) {
     nlua_print(lstate);
     return ERROR_OTHER;
   }
@@ -1546,6 +1529,8 @@ static int regex_match_line(lua_State *lstate)
   return nret;
 }
 
+// Required functions for lua c functions as VimL callbacks
+
 int nlua_CFunction_func_call(
     int argcount,
     typval_T *argvars,
@@ -1555,53 +1540,40 @@ int nlua_CFunction_func_call(
     lua_State *const lstate = nlua_enter();
     LuaCFunctionState *funcstate = (LuaCFunctionState *)state;
 
-    return typval_exec_lua_callable(
+    return typval_exec_lua_callable(lstate, funcstate->lua_callable,
-        lstate,
+                                    argcount, argvars, rettv);
-        funcstate->lua_callable,
-        argcount,
-        argvars,
-        rettv);
 }
-/// Required functions for lua c functions as VimL callbacks
+
 void nlua_CFunction_func_free(void *state)
 {
     lua_State *const lstate = nlua_enter();
     LuaCFunctionState *funcstate = (LuaCFunctionState *)state;
 
     nlua_unref(lstate, funcstate->lua_callable.func_ref);
-    nlua_unref(lstate, funcstate->lua_callable.table_ref);
     xfree(funcstate);
 }
 
 bool nlua_is_table_from_lua(typval_T *const arg)
 {
-  if (arg->v_type != VAR_DICT && arg->v_type != VAR_LIST) {
-    return false;
-  }
-
   if (arg->v_type == VAR_DICT) {
-    return arg->vval.v_dict->lua_table_ref > 0
+    return arg->vval.v_dict->lua_table_ref != LUA_NOREF;
-      && arg->vval.v_dict->lua_table_ref != LUA_NOREF;
   } else if (arg->v_type == VAR_LIST) {
-    return arg->vval.v_list->lua_table_ref > 0
+    return arg->vval.v_list->lua_table_ref != LUA_NOREF;
-      && arg->vval.v_list->lua_table_ref != LUA_NOREF;
+  } else {
-  }
-
     return false;
   }
+}
 
 char_u *nlua_register_table_as_callable(typval_T *const arg)
 {
-  if (!nlua_is_table_from_lua(arg)) {
+  LuaRef table_ref = LUA_NOREF;
-    return NULL;
-  }
-
-  LuaRef table_ref;
   if (arg->v_type == VAR_DICT) {
     table_ref = arg->vval.v_dict->lua_table_ref;
   } else if (arg->v_type == VAR_LIST) {
     table_ref = arg->vval.v_list->lua_table_ref;
-  } else {
+  }
+
+  if (table_ref == LUA_NOREF) {
     return NULL;
   }
 
@@ -1611,55 +1583,34 @@ char_u *nlua_register_table_as_callable(typval_T *const arg)
   int top = lua_gettop(lstate);
 #endif
 
-  nlua_pushref(lstate, table_ref);
+  nlua_pushref(lstate, table_ref);  // [table]
   if (!lua_getmetatable(lstate, -1)) {
+    lua_pop(lstate, 1);
+    assert(top == lua_gettop(lstate));
     return NULL;
-  }
+  }  // [table, mt]
 
-  lua_getfield(lstate, -1, "__call");
+  lua_getfield(lstate, -1, "__call");  // [table, mt, mt.__call]
   if (!lua_isfunction(lstate, -1)) {
+    lua_pop(lstate, 3);
+    assert(top == lua_gettop(lstate));
     return NULL;
   }
-
+  lua_pop(lstate, 2);  // [table]
-  LuaRef new_table_ref = nlua_newref(lstate, table_ref);
 
   LuaCFunctionState *state = xmalloc(sizeof(LuaCFunctionState));
   state->lua_callable.func_ref = nlua_ref(lstate, -1);
-  state->lua_callable.table_ref = new_table_ref;
 
-  char_u *name = register_cfunc(
+  char_u *name = register_cfunc(&nlua_CFunction_func_call,
-      &nlua_CFunction_func_call,
+                                &nlua_CFunction_func_free, state);
-      &nlua_CFunction_func_free,
-      state);
 
 
-  lua_pop(lstate, 3);
+  lua_pop(lstate, 1);  // []
   assert(top == lua_gettop(lstate));
 
   return name;
 }
 
-/// Helper function to free a list_T
-void nlua_free_typval_list(list_T *const l)
-{
-  if (l->lua_table_ref != LUA_NOREF && l->lua_table_ref > 0) {
-    lua_State *const lstate = nlua_enter();
-    nlua_unref(lstate, l->lua_table_ref);
-    l->lua_table_ref = LUA_NOREF;
-  }
-}
-
-
-/// Helper function to free a dict_T
-void nlua_free_typval_dict(dict_T *const d)
-{
-  if (d->lua_table_ref != LUA_NOREF && d->lua_table_ref > 0) {
-    lua_State *const lstate = nlua_enter();
-    nlua_unref(lstate, d->lua_table_ref);
-    d->lua_table_ref = LUA_NOREF;
-  }
-}
-
 void nlua_execute_log_keystroke(int c)
 {
   char_u buf[NUMBUFLEN];