patch 9.0.2038: Vim9: object method funcref not cleaned up after use

Problem:  Vim9: object method funcref not cleaned up after use
Solution: Clean up type stack after using object method funcref,
          remove now longer used ISN_DEFEROBJ instrunction

closes: #13360

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>

src/proto/vim9instr.pro

@@ -62,7 +62,7 @@ int generate_CALL(cctx_T *cctx, ufunc_T *ufunc, class_T *cl, int mi, int pushed_
 int generate_UCALL(cctx_T *cctx, char_u *name, int argcount);
 int check_func_args_from_type(cctx_T *cctx, type_T *type, int argcount, int at_top, char_u *name);
 int generate_PCALL(cctx_T *cctx, int argcount, char_u *name, type_T *type, int at_top);
-int generate_DEFER(cctx_T *cctx, int var_idx, int obj_method, int argcount);
+int generate_DEFER(cctx_T *cctx, int var_idx, int argcount);
 int generate_STRINGMEMBER(cctx_T *cctx, char_u *name, size_t len);
 int generate_ECHO(cctx_T *cctx, int with_white, int count);
 int generate_MULT_EXPR(cctx_T *cctx, isntype_T isn_type, int count);

src/testdir/test_vim9_class.vim

@@ -8022,4 +8022,256 @@ def Test_class_member_funcref()
   v9.CheckSourceSuccess(lines)
 enddef
 
+" Test for using object methods as popup callback functions
+def Test_objmethod_popup_callback()
+  # Use the popup from the script level
+  var lines =<< trim END
+    vim9script
+
+    class A
+      this.selection: number = -1
+      this.filterkeys: list<string> = []
+
+      def PopupFilter(id: number, key: string): bool
+        add(this.filterkeys, key)
+        return popup_filter_yesno(id, key)
+      enddef
+
+      def PopupCb(id: number, result: number)
+        this.selection = result ? 100 : 200
+      enddef
+    endclass
+
+    var a = A.new()
+    feedkeys('', 'xt')
+    var winid = popup_create('Y/N?',
+                              {filter: a.PopupFilter, callback: a.PopupCb})
+    feedkeys('y', 'xt')
+    popup_close(winid)
+    assert_equal(100, a.selection)
+    assert_equal(['y'], a.filterkeys)
+    feedkeys('', 'xt')
+    winid = popup_create('Y/N?',
+                              {filter: a.PopupFilter, callback: a.PopupCb})
+    feedkeys('n', 'xt')
+    popup_close(winid)
+    assert_equal(200, a.selection)
+    assert_equal(['y', 'n'], a.filterkeys)
+  END
+  v9.CheckSourceSuccess(lines)
+
+  # Use the popup from a def function
+  lines =<< trim END
+    vim9script
+
+    class A
+      this.selection: number = -1
+      this.filterkeys: list<string> = []
+
+      def PopupFilter(id: number, key: string): bool
+        add(this.filterkeys, key)
+        return popup_filter_yesno(id, key)
+      enddef
+
+      def PopupCb(id: number, result: number)
+        this.selection = result ? 100 : 200
+      enddef
+    endclass
+
+    def Foo()
+      var a = A.new()
+      feedkeys('', 'xt')
+      var winid = popup_create('Y/N?',
+                                {filter: a.PopupFilter, callback: a.PopupCb})
+      feedkeys('y', 'xt')
+      popup_close(winid)
+      assert_equal(100, a.selection)
+      assert_equal(['y'], a.filterkeys)
+      feedkeys('', 'xt')
+      winid = popup_create('Y/N?',
+                                {filter: a.PopupFilter, callback: a.PopupCb})
+      feedkeys('n', 'xt')
+      popup_close(winid)
+      assert_equal(200, a.selection)
+      assert_equal(['y', 'n'], a.filterkeys)
+    enddef
+    Foo()
+  END
+  v9.CheckSourceSuccess(lines)
+enddef
+
+" Test for using class methods as popup callback functions
+def Test_classmethod_popup_callback()
+  # Use the popup from the script level
+  var lines =<< trim END
+    vim9script
+
+    class A
+      static selection: number = -1
+      static filterkeys: list<string> = []
+
+      static def PopupFilter(id: number, key: string): bool
+        add(filterkeys, key)
+        return popup_filter_yesno(id, key)
+      enddef
+
+      static def PopupCb(id: number, result: number)
+        selection = result ? 100 : 200
+      enddef
+    endclass
+
+    feedkeys('', 'xt')
+    var winid = popup_create('Y/N?',
+                              {filter: A.PopupFilter, callback: A.PopupCb})
+    feedkeys('y', 'xt')
+    popup_close(winid)
+    assert_equal(100, A.selection)
+    assert_equal(['y'], A.filterkeys)
+    feedkeys('', 'xt')
+    winid = popup_create('Y/N?',
+                              {filter: A.PopupFilter, callback: A.PopupCb})
+    feedkeys('n', 'xt')
+    popup_close(winid)
+    assert_equal(200, A.selection)
+    assert_equal(['y', 'n'], A.filterkeys)
+  END
+  v9.CheckSourceSuccess(lines)
+
+  # Use the popup from a def function
+  lines =<< trim END
+    vim9script
+
+    class A
+      static selection: number = -1
+      static filterkeys: list<string> = []
+
+      static def PopupFilter(id: number, key: string): bool
+        add(filterkeys, key)
+        return popup_filter_yesno(id, key)
+      enddef
+
+      static def PopupCb(id: number, result: number)
+        selection = result ? 100 : 200
+      enddef
+    endclass
+
+    def Foo()
+      feedkeys('', 'xt')
+      var winid = popup_create('Y/N?',
+                                {filter: A.PopupFilter, callback: A.PopupCb})
+      feedkeys('y', 'xt')
+      popup_close(winid)
+      assert_equal(100, A.selection)
+      assert_equal(['y'], A.filterkeys)
+      feedkeys('', 'xt')
+      winid = popup_create('Y/N?',
+                                {filter: A.PopupFilter, callback: A.PopupCb})
+      feedkeys('n', 'xt')
+      popup_close(winid)
+      assert_equal(200, A.selection)
+      assert_equal(['y', 'n'], A.filterkeys)
+    enddef
+    Foo()
+  END
+  v9.CheckSourceSuccess(lines)
+enddef
+
+" Test for using an object method as a timer callback function
+def Test_objmethod_timer_callback()
+  # Use the timer callback from script level
+  var lines =<< trim END
+    vim9script
+
+    class A
+      this.timerTick: number = -1
+      def TimerCb(timerID: number)
+        this.timerTick = 6
+      enddef
+    endclass
+
+    var a = A.new()
+    timer_start(0, a.TimerCb)
+    var maxWait = 5
+    while maxWait > 0 && a.timerTick == -1
+      :sleep 10m
+      maxWait -= 1
+    endwhile
+    assert_equal(6, a.timerTick)
+  END
+  v9.CheckSourceSuccess(lines)
+
+  # Use the timer callback from a def function
+  lines =<< trim END
+    vim9script
+
+    class A
+      this.timerTick: number = -1
+      def TimerCb(timerID: number)
+        this.timerTick = 6
+      enddef
+    endclass
+
+    def Foo()
+      var a = A.new()
+      timer_start(0, a.TimerCb)
+      var maxWait = 5
+      while maxWait > 0 && a.timerTick == -1
+        :sleep 10m
+        maxWait -= 1
+      endwhile
+      assert_equal(6, a.timerTick)
+    enddef
+    Foo()
+  END
+  v9.CheckSourceSuccess(lines)
+enddef
+
+" Test for using a class method as a timer callback function
+def Test_classmethod_timer_callback()
+  # Use the timer callback from script level
+  var lines =<< trim END
+    vim9script
+
+    class A
+      static timerTick: number = -1
+      static def TimerCb(timerID: number)
+        timerTick = 6
+      enddef
+    endclass
+
+    timer_start(0, A.TimerCb)
+    var maxWait = 5
+    while maxWait > 0 && A.timerTick == -1
+      :sleep 10m
+      maxWait -= 1
+    endwhile
+    assert_equal(6, A.timerTick)
+  END
+  v9.CheckSourceSuccess(lines)
+
+  # Use the timer callback from a def function
+  lines =<< trim END
+    vim9script
+
+    class A
+      static timerTick: number = -1
+      static def TimerCb(timerID: number)
+        timerTick = 6
+      enddef
+    endclass
+
+    def Foo()
+      timer_start(0, A.TimerCb)
+      var maxWait = 5
+      while maxWait > 0 && A.timerTick == -1
+        :sleep 10m
+        maxWait -= 1
+      endwhile
+      assert_equal(6, A.timerTick)
+    enddef
+    Foo()
+  END
+  v9.CheckSourceSuccess(lines)
+enddef
+
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

src/testdir/test_vim9_disassemble.vim

@@ -3276,7 +3276,7 @@ def Test_funcref_with_class()
     'defer a.Foo()\_s*' ..
     '0 LOAD arg\[-1\]\_s*' ..
     '1 FUNCREF A.Foo\_s*' ..
-    '2 DEFEROBJ 0 args\_s*' ..
+    '2 DEFER 0 args\_s*' ..
     '3 RETURN void', g:instr)
   unlet g:instr
 enddef

src/version.c

@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2038,
 /**/
     2037,
 /**/

src/vim9.h

@@ -125,7 +125,6 @@ typedef enum {
     ISN_NEWFUNC,    // create a global function from a lambda function
     ISN_DEF,	    // list functions
     ISN_DEFER,	    // :defer  argument count is isn_arg.number
-    ISN_DEFEROBJ,   // idem, function is an object method
 
     // expression operations
     ISN_JUMP,	    // jump if condition is matched isn_arg.jump

src/vim9cmds.c

@@ -2000,7 +2000,6 @@ compile_defer(char_u *arg_start, cctx_T *cctx)
     int		defer_var_idx;
     type_T	*type;
     int		func_idx;
-    int		obj_method = 0;
 
     // Get a funcref for the function name.
     // TODO: better way to find the "(".
@@ -2016,15 +2015,8 @@ compile_defer(char_u *arg_start, cctx_T *cctx)
 	// TODO: better type
 	generate_PUSHFUNC(cctx, (char_u *)internal_func_name(func_idx),
 							   &t_func_any, FALSE);
-    else
+    else if (compile_expr0(&arg, cctx) == FAIL)
-    {
-	int typecount = cctx->ctx_type_stack.ga_len;
-	if (compile_expr0(&arg, cctx) == FAIL)
 	return NULL;
-	if (cctx->ctx_type_stack.ga_len >= typecount + 2)
-	    // must have seen "obj.Func", pushed an object and a function
-	    obj_method = 1;
-    }
     *paren = '(';
 
     // check for function type
@@ -2056,7 +2048,7 @@ compile_defer(char_u *arg_start, cctx_T *cctx)
     defer_var_idx = get_defer_var_idx(cctx);
     if (defer_var_idx == 0)
 	return NULL;
-    if (generate_DEFER(cctx, defer_var_idx - 1, obj_method, argcount) == FAIL)
+    if (generate_DEFER(cctx, defer_var_idx - 1, argcount) == FAIL)
 	return NULL;
 
     return skipwhite(arg);

src/vim9execute.c

@@ -1029,10 +1029,9 @@ add_defer_item(int var_idx, int argcount, ectx_T *ectx)
  * Returns OK or FAIL.
  */
     static int
-defer_command(int var_idx, int has_obj, int argcount, ectx_T *ectx)
+defer_command(int var_idx, int argcount, ectx_T *ectx)
 {
-    int		obj_off = has_obj ? 1 : 0;
+    list_T	*l = add_defer_item(var_idx, argcount, ectx);
-    list_T	*l = add_defer_item(var_idx, argcount + obj_off, ectx);
     int		i;
     typval_T	*func_tv;
 
@@ -1040,20 +1039,18 @@ defer_command(int var_idx, int has_obj, int argcount, ectx_T *ectx)
 	return FAIL;
 
     func_tv = STACK_TV_BOT(-argcount - 1);
-    if (has_obj ? func_tv->v_type != VAR_PARTIAL : func_tv->v_type != VAR_FUNC)
+    if (func_tv->v_type != VAR_PARTIAL && func_tv->v_type != VAR_FUNC)
     {
 	semsg(_(e_expected_str_but_got_str),
-		has_obj ? "partial" : "function",
+		"function or partial",
 		vartype_name(func_tv->v_type));
 	return FAIL;
     }
     list_set_item(l, 0, func_tv);
-    if (has_obj)
-	list_set_item(l, 1, STACK_TV_BOT(-argcount - 2));
 
     for (i = 0; i < argcount; ++i)
-	list_set_item(l, i + 1 + obj_off, STACK_TV_BOT(-argcount + i));
+	list_set_item(l, i + 1, STACK_TV_BOT(-argcount + i));
-    ectx->ec_stack.ga_len -= argcount + 1 + obj_off;
+    ectx->ec_stack.ga_len -= argcount + 1;
     return OK;
 }
 
@@ -1116,15 +1113,12 @@ invoke_defer_funcs(ectx_T *ectx)
 	int	    i;
 	listitem_T  *arg_li = l->lv_first;
 	typval_T    *functv = &l->lv_first->li_tv;
-	int	    obj_off = functv->v_type == VAR_PARTIAL ? 1 : 0;
+	int	    argcount = l->lv_len - 1;
-	int	    argcount = l->lv_len - 1 - obj_off;
 
 	if (functv->vval.v_string == NULL)
 	    // already being called, can happen if function does ":qa"
 	    continue;
 
-	if (obj_off == 1)
-	    arg_li = arg_li->li_next;  // second list item is the object
 	for (i = 0; i < argcount; ++i)
 	{
 	    arg_li = arg_li->li_next;
@@ -1138,7 +1132,7 @@ invoke_defer_funcs(ectx_T *ectx)
 	if (functv->v_type == VAR_PARTIAL)
 	{
 	    funcexe.fe_partial = functv->vval.v_partial;
-	    funcexe.fe_object = l->lv_first->li_next->li_tv.vval.v_object;
+	    funcexe.fe_object = functv->vval.v_partial->pt_obj;
 	    if (funcexe.fe_object != NULL)
 		++funcexe.fe_object->obj_refcount;
 	}
@@ -4401,9 +4395,7 @@ exec_instructions(ectx_T *ectx)
 
 	    // :defer func(arg)
 	    case ISN_DEFER:
-	    case ISN_DEFEROBJ:
 		if (defer_command(iptr->isn_arg.defer.defer_var_idx,
-			     iptr->isn_type == ISN_DEFEROBJ,
 			     iptr->isn_arg.defer.defer_argcount, ectx) == FAIL)
 		    goto on_error;
 		break;
@@ -6933,9 +6925,7 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
 		smsg("%s%4d PCALL end", pfx, current);
 		break;
 	    case ISN_DEFER:
-	    case ISN_DEFEROBJ:
+		smsg("%s%4d DEFER %d args", pfx, current,
-		smsg("%s%4d %s %d args", pfx, current,
-			    iptr->isn_type == ISN_DEFER ? "DEFER" : "DEFEROBJ",
 				      (int)iptr->isn_arg.defer.defer_argcount);
 		break;
 	    case ISN_RETURN:

src/vim9expr.c

@@ -450,9 +450,9 @@ compile_class_object_index(cctx_T *cctx, char_u **arg, type_T *type)
 		return FAIL;
 	    }
 	    *arg = name_end;
-	    if (type->tt_type == VAR_OBJECT)
+	    // Remove the object type from the stack
+	    --cctx->ctx_type_stack.ga_len;
 	    return generate_FUNCREF(cctx, fp, cl, TRUE, m_idx, NULL);
-	    return generate_FUNCREF(cctx, fp, NULL, FALSE, 0, NULL);
 	}
 
 	member_not_found_msg(cl, VAR_OBJECT, name, len);
@@ -490,9 +490,9 @@ compile_class_object_index(cctx_T *cctx, char_u **arg, type_T *type)
 		return FAIL;
 	    }
 	    *arg = name_end;
-	    if (type->tt_type == VAR_CLASS)
+	    // Remove the class type from the stack
+	    --cctx->ctx_type_stack.ga_len;
 	    return generate_FUNCREF(cctx, fp, cl, FALSE, m_idx, NULL);
-	    return generate_FUNCREF(cctx, fp, NULL, FALSE, 0, NULL);
 	}
 
 	member_not_found_msg(cl, VAR_CLASS, name, len);

src/vim9instr.c

@@ -2039,17 +2039,14 @@ generate_PCALL(
 
 /*
  * Generate an ISN_DEFER instruction.
- * "obj_method" is one for "obj.Method()", zero otherwise.
  */
     int
-generate_DEFER(cctx_T *cctx, int var_idx, int obj_method, int argcount)
+generate_DEFER(cctx_T *cctx, int var_idx, int argcount)
 {
     isn_T *isn;
 
     RETURN_OK_IF_SKIP(cctx);
-    if ((isn = generate_instr_drop(cctx,
+    if ((isn = generate_instr_drop(cctx, ISN_DEFER, argcount + 1)) == NULL)
-		    obj_method == 0 ? ISN_DEFER : ISN_DEFEROBJ,
-		    argcount + 1)) == NULL)
 	return FAIL;
     isn->isn_arg.defer.defer_var_idx = var_idx;
     isn->isn_arg.defer.defer_argcount = argcount;
@@ -2711,7 +2708,6 @@ delete_instr(isn_T *isn)
 	case ISN_COND2BOOL:
 	case ISN_DEBUG:
 	case ISN_DEFER:
-	case ISN_DEFEROBJ:
 	case ISN_DROP:
 	case ISN_ECHO:
 	case ISN_ECHOCONSOLE: