vim-patch:9.0.0370: cleaning up afterwards can make a function messy

Problem:    Cleaning up afterwards can make a function messy.
Solution:   Add the :defer command.

1d84f7608f

Omit EX_EXPR_ARG: Vim9 script only.
Make :def throw E319 to avoid confusing behavior.

Co-authored-by: Bram Moolenaar <Bram@vim.org>

runtime/doc/userfunc.txt

@@ -350,10 +350,67 @@ A function can also be called as part of evaluating an expression or when it
 is used as a method: >
 	let x = GetList()
 	let y = GetList()->Filter()
+<
+==============================================================================
 
+3. Cleaning up in a function ~
+							*:defer*
+:defer {func}({args})	Call {func} when the current function is done.
+			{args} are evaluated here.
+
+Quite often a command in a function has a global effect, which must be undone
+when the function finishes.  Handling this in all kinds of situations can be a
+hassle.  Especially when an unexpected error is encountered.  This can be done
+with `try` / `finally` blocks, but this gets complicated when there is more
+than one.
+
+A much simpler solution is using `defer`.  It schedules a function call when
+the function is returning, no matter if there is an error.  Example: >
+	func Filter(text) abort
+	  call writefile(a:text, 'Tempfile')
+	  call system('filter < Tempfile > Outfile')
+	  call Handle('Outfile')
+	  call delete('Tempfile')
+	  call delete('Outfile')
+	endfunc
+
+Here 'Tempfile' and 'Outfile' will not be deleted if something causes the
+function to abort.  `:defer` can be used to avoid that: >
+	func Filter(text) abort
+	  call writefile(a:text, 'Tempfile')
+	  defer delete('Tempfile')
+	  defer delete('Outfile')
+	  call system('filter < Tempfile > Outfile')
+	  call Handle('Outfile')
+	endfunc
+
+Note that deleting "Outfile" is scheduled before calling `system()`, since it
+can be created even when `system()` fails.
+
+The deferred functions are called in reverse order, the last one added is
+executed first.  A useless example: >
+	func Useless() abort
+	  for s in range(3)
+	    defer execute('echomsg "number ' .. s .. '"')
+	  endfor
+	endfunc
+
+Now `:messages` shows:
+	number 2
+	number 1
+	number 0
+
+Any return value of the deferred function is discarded.  The function cannot
+be followed by anything, such as "->func" or ".member".  Currently `:defer
+GetArg()->TheFunc()` does not work, it may work in a later version.
+
+Errors are reported but do not cause aborting execution of deferred functions.
+
+No range is accepted.
 
 ==============================================================================
-3. Automatically loading functions ~
+
+4. Automatically loading functions ~
 							*autoload-functions*
 When using many or large functions, it's possible to automatically define them
 only when they are used.  There are two methods: with an autocommand and with

src/nvim/eval/typval_defs.h

@@ -299,6 +299,7 @@ struct funccall_S {
   linenr_T breakpoint;  ///< Next line with breakpoint or zero.
   int dbg_tick;  ///< debug_tick when breakpoint was set.
   int level;  ///< Top nesting level of executed function.
+  garray_T fc_defer;  ///< Functions to be called on return.
   proftime_T prof_child;  ///< Time spent in a child.
   funccall_T *caller;  ///< Calling function or NULL; or next funccal in
                        ///< list pointed to by previous_funccal.

src/nvim/eval/userfunc.c

@@ -53,6 +53,13 @@
 # include "eval/userfunc.c.generated.h"
 #endif
 
+/// structure used as item in "fc_defer"
+typedef struct {
+  char *dr_name;  ///< function name, allocated
+  typval_T dr_argvars[MAX_FUNC_ARGS + 1];
+  int dr_argcount;
+} defer_T;
+
 static hashtab_T func_hashtab;
 
 // Used by get_func_tv()
@@ -469,7 +476,42 @@ void emsg_funcname(const char *errmsg, const char *name)
   }
 }
 
-/// Allocate a variable for the result of a function.
+/// Get function arguments at "*arg" and advance it.
+/// Return them in "*argvars[MAX_FUNC_ARGS + 1]" and the count in "argcount".
+static int get_func_arguments(char **arg, evalarg_T *const evalarg, int partial_argc,
+                              typval_T *argvars, int *argcount)
+{
+  char *argp = *arg;
+  int ret = OK;
+
+  // Get the arguments.
+  while (*argcount < MAX_FUNC_ARGS - partial_argc) {
+    argp = skipwhite(argp + 1);             // skip the '(' or ','
+
+    if (*argp == ')' || *argp == ',' || *argp == NUL) {
+      break;
+    }
+    if (eval1(&argp, &argvars[*argcount], evalarg) == FAIL) {
+      ret = FAIL;
+      break;
+    }
+    (*argcount)++;
+    if (*argp != ',') {
+      break;
+    }
+  }
+
+  argp = skipwhite(argp);
+  if (*argp == ')') {
+    argp++;
+  } else {
+    ret = FAIL;
+  }
+  *arg = argp;
+  return ret;
+}
+
+/// Call a function and put the result in "rettv".
 ///
 /// @param name  name of the function
 /// @param len  length of "name" or -1 to use strlen()
@@ -480,34 +522,16 @@ void emsg_funcname(const char *errmsg, const char *name)
 int get_func_tv(const char *name, int len, typval_T *rettv, char **arg, evalarg_T *const evalarg,
                 funcexe_T *funcexe)
 {
-  char *argp;
-  int ret = OK;
   typval_T argvars[MAX_FUNC_ARGS + 1];          // vars for arguments
   int argcount = 0;                     // number of arguments found
   const bool evaluate = evalarg == NULL ? false : (evalarg->eval_flags & EVAL_EVALUATE);
 
-  // Get the arguments.
-  argp = *arg;
-  while (argcount < MAX_FUNC_ARGS
-         - (funcexe->fe_partial == NULL ? 0 : funcexe->fe_partial->pt_argc)) {
-    argp = skipwhite(argp + 1);             // skip the '(' or ','
-    if (*argp == ')' || *argp == ',' || *argp == NUL) {
-      break;
-    }
-    if (eval1(&argp, &argvars[argcount], evalarg) == FAIL) {
-      ret = FAIL;
-      break;
-    }
-    argcount++;
-    if (*argp != ',') {
-      break;
-    }
-  }
-  if (*argp == ')') {
-    argp++;
-  } else {
-    ret = FAIL;
-  }
+  char *argp = *arg;
+  int ret = get_func_arguments(&argp, evalarg,
+                               (funcexe->fe_partial == NULL
+                                ? 0
+                                : funcexe->fe_partial->pt_argc),
+                               argvars, &argcount);
 
   if (ret == OK) {
     int i = 0;
@@ -1148,6 +1172,9 @@ void call_user_func(ufunc_T *fp, int argcount, typval_T *argvars, typval_T *rett
                DOCMD_NOWAIT|DOCMD_VERBOSE|DOCMD_REPEAT);
   }
 
+  // Invoke functions added with ":defer".
+  handle_defer();
+
   RedrawingDisabled--;
 
   // when the function was aborted because of an error, return -1
@@ -1544,7 +1571,7 @@ int call_func(const char *funcname, int len, typval_T *rettv, int argcount_in, t
   int argv_base = 0;
   partial_T *partial = funcexe->fe_partial;
 
-  // Initialize rettv so that it is safe for caller to invoke clear_tv(rettv)
+  // Initialize rettv so that it is safe for caller to invoke tv_clear(rettv)
   // even when call_func() returns FAIL.
   rettv->v_type = VAR_UNKNOWN;
 
@@ -3033,7 +3060,115 @@ void ex_return(exarg_T *eap)
   clear_evalarg(&evalarg, eap);
 }
 
+static int ex_call_inner(exarg_T *eap, char *name, char **arg, char *startarg,
+                         funcexe_T *funcexe_init, evalarg_T *const evalarg)
+{
+  bool doesrange;
+  bool failed = false;
+
+  for (linenr_T lnum = eap->line1; lnum <= eap->line2; lnum++) {
+    if (eap->addr_count > 0) {  // -V560
+      if (lnum > curbuf->b_ml.ml_line_count) {
+        // If the function deleted lines or switched to another buffer
+        // the line number may become invalid.
+        emsg(_(e_invrange));
+        break;
+      }
+      curwin->w_cursor.lnum = lnum;
+      curwin->w_cursor.col = 0;
+      curwin->w_cursor.coladd = 0;
+    }
+    *arg = startarg;
+
+    funcexe_T funcexe = *funcexe_init;
+    funcexe.fe_doesrange = &doesrange;
+    typval_T rettv;
+    rettv.v_type = VAR_UNKNOWN;  // tv_clear() uses this
+    if (get_func_tv(name, -1, &rettv, arg, evalarg, &funcexe) == FAIL) {
+      failed = true;
+      break;
+    }
+
+    // Handle a function returning a Funcref, Dictionary or List.
+    if (handle_subscript((const char **)arg, &rettv, &EVALARG_EVALUATE, true) == FAIL) {
+      failed = true;
+      break;
+    }
+
+    tv_clear(&rettv);
+    if (doesrange) {
+      break;
+    }
+
+    // Stop when immediately aborting on error, or when an interrupt
+    // occurred or an exception was thrown but not caught.
+    // get_func_tv() returned OK, so that the check for trailing
+    // characters below is executed.
+    if (aborting()) {
+      break;
+    }
+  }
+
+  return failed;
+}
+
+/// Core part of ":defer func(arg)".  "arg" points to the "(" and is advanced.
+///
+/// @return  FAIL or OK.
+static int ex_defer_inner(char *name, char **arg, evalarg_T *const evalarg)
+{
+  typval_T argvars[MAX_FUNC_ARGS + 1];  // vars for arguments
+  int argcount = 0;  // number of arguments found
+  int ret = FAIL;
+
+  if (current_funccal == NULL) {
+    semsg(_(e_str_not_inside_function), "defer");
+    return FAIL;
+  }
+  if (get_func_arguments(arg, evalarg, false, argvars, &argcount) == FAIL) {
+    goto theend;
+  }
+  char *saved_name = xstrdup(name);
+
+  if (current_funccal->fc_defer.ga_itemsize == 0) {
+    ga_init(&current_funccal->fc_defer, sizeof(defer_T), 10);
+  }
+  defer_T *dr = GA_APPEND_VIA_PTR(defer_T, &current_funccal->fc_defer);
+  dr->dr_name = saved_name;
+  dr->dr_argcount = argcount;
+  while (argcount > 0) {
+    argcount--;
+    dr->dr_argvars[argcount] = argvars[argcount];
+  }
+  ret = OK;
+
+theend:
+  while (--argcount >= 0) {
+    tv_clear(&argvars[argcount]);
+  }
+  return ret;
+}
+
+/// Invoked after a function has finished: invoke ":defer" functions.
+static void handle_defer(void)
+{
+  for (int idx = current_funccal->fc_defer.ga_len - 1; idx >= 0; idx--) {
+    defer_T *dr = ((defer_T *)current_funccal->fc_defer.ga_data) + idx;
+    funcexe_T funcexe = { .fe_evaluate = true };
+    typval_T rettv;
+    rettv.v_type = VAR_UNKNOWN;     // tv_clear() uses this
+    call_func(dr->dr_name, -1, &rettv, dr->dr_argcount, dr->dr_argvars, &funcexe);
+    tv_clear(&rettv);
+    xfree(dr->dr_name);
+    for (int i = dr->dr_argcount - 1; i >= 0; i--) {
+      tv_clear(&dr->dr_argvars[i]);
+    }
+  }
+  ga_clear(&current_funccal->fc_defer);
+}
+
 /// ":1,25call func(arg1, arg2)" function call.
+/// ":defer func(arg1, arg2)"    deferred function call.
 void ex_call(exarg_T *eap)
 {
   char *arg = eap->arg;
@@ -3041,9 +3176,6 @@ void ex_call(exarg_T *eap)
   char *name;
   char *tofree;
   int len;
-  typval_T rettv;
-  linenr_T lnum;
-  bool doesrange;
   bool failed = false;
   funcdict_T fudi;
   partial_T *partial = NULL;
@@ -3051,6 +3183,7 @@ void ex_call(exarg_T *eap)
 
   fill_evalarg_from_eap(&evalarg, eap, eap->skip);
   if (eap->skip) {
+    typval_T rettv;
     // trans_function_name() doesn't work well when skipping, use eval0()
     // instead to skip to any following command, e.g. for:
     //   :if 0 | call dict.foo().bar() | endif.
@@ -3089,59 +3222,24 @@ void ex_call(exarg_T *eap)
   // Skip white space to allow ":call func ()".  Not good, but required for
   // backward compatibility.
   startarg = skipwhite(arg);
-  rettv.v_type = VAR_UNKNOWN;  // tv_clear() uses this.
 
   if (*startarg != '(') {
     semsg(_(e_missingparen), eap->arg);
     goto end;
   }
 
-  lnum = eap->line1;
-  for (; lnum <= eap->line2; lnum++) {
-    if (eap->addr_count > 0) {  // -V560
-      if (lnum > curbuf->b_ml.ml_line_count) {
-        // If the function deleted lines or switched to another buffer
-        // the line number may become invalid.
-        emsg(_(e_invrange));
-        break;
-      }
-      curwin->w_cursor.lnum = lnum;
-      curwin->w_cursor.col = 0;
-      curwin->w_cursor.coladd = 0;
-    }
+  if (eap->cmdidx == CMD_defer) {
     arg = startarg;
-
+    failed = ex_defer_inner(name, &arg, &evalarg) == FAIL;
+  } else {
     funcexe_T funcexe = FUNCEXE_INIT;
-    funcexe.fe_firstline = eap->line1;
-    funcexe.fe_lastline = eap->line2;
-    funcexe.fe_doesrange = &doesrange;
-    funcexe.fe_evaluate = true;
     funcexe.fe_partial = partial;
     funcexe.fe_selfdict = fudi.fd_dict;
+    funcexe.fe_firstline = eap->line1;
+    funcexe.fe_lastline = eap->line2;
     funcexe.fe_found_var = found_var;
-    if (get_func_tv(name, -1, &rettv, &arg, &evalarg, &funcexe) == FAIL) {
-      failed = true;
-      break;
-    }
-
-    // Handle a function returning a Funcref, Dictionary or List.
-    if (handle_subscript((const char **)&arg, &rettv, &EVALARG_EVALUATE, true) == FAIL) {
-      failed = true;
-      break;
-    }
-
-    tv_clear(&rettv);
-    if (doesrange) {
-      break;
-    }
-
-    // Stop when immediately aborting on error, or when an interrupt
-    // occurred or an exception was thrown but not caught.
-    // get_func_tv() returned OK, so that the check for trailing
-    // characters below is executed.
-    if (aborting()) {
-      break;
-    }
+    funcexe.fe_evaluate = true;
+    failed = ex_call_inner(eap, name, &arg, startarg, &funcexe, &evalarg);
   }
 
   // When inside :try we need to check for following "| catch" or "| endtry".

src/nvim/ex_cmds.lua

@@ -714,6 +714,18 @@ module.cmds = {
     addr_type='ADDR_OTHER',
     func='ex_debuggreedy',
   },
+  {
+    command='def',
+    flags=bit.bor(EXTRA, BANG, SBOXOK, CMDWIN, LOCK_OK),
+    addr_type='ADDR_NONE',
+    func='ex_ni',
+  },
+  {
+    command='defer',
+    flags=bit.bor(NEEDARG, EXTRA, NOTRLCOM, CMDWIN, LOCK_OK),
+    addr_type='ADDR_NONE',
+    func='ex_call',
+  },
   {
     command='delcommand',
     flags=bit.bor(BANG, NEEDARG, WORD1, TRLBAR, CMDWIN, LOCK_OK),

src/nvim/ex_eval.c

@@ -1966,7 +1966,7 @@ void rewind_conditionals(cstack_T *cstack, int idx, int cond_type, int *cond_lev
 /// Handle ":endfunction" when not after a ":function"
 void ex_endfunction(exarg_T *eap)
 {
-  emsg(_("E193: :endfunction not inside a function"));
+  semsg(_(e_str_not_inside_function), ":endfunction");
 }
 
 /// @return  true if the string "p" looks like a ":while" or ":for" command.

src/nvim/globals.h

@@ -986,6 +986,8 @@ EXTERN const char e_maxmempat[] INIT(= N_("E363: pattern uses more memory than '
 EXTERN const char e_emptybuf[] INIT(= N_("E749: empty buffer"));
 EXTERN const char e_nobufnr[] INIT(= N_("E86: Buffer %" PRId64 " does not exist"));
 
+EXTERN const char e_str_not_inside_function[] INIT(= N_("E193: %s not inside a function"));
+
 EXTERN const char e_invalpat[] INIT(= N_("E682: Invalid search pattern or delimiter"));
 EXTERN const char e_bufloaded[] INIT(= N_("E139: File is loaded in another buffer"));
 EXTERN const char e_notset[] INIT(= N_("E764: Option '%s' is not set"));

test/old/testdir/test_user_func.vim

@@ -532,4 +532,36 @@ func Test_funcdef_alloc_failure()
   bw!
 endfunc
 
+func AddDefer(arg)
+  call extend(g:deferred, [a:arg])
+endfunc
+
+func WithDeferTwo()
+  call extend(g:deferred, ['in Two'])
+  for nr in range(3)
+    defer AddDefer('Two' .. nr)
+  endfor
+  call extend(g:deferred, ['end Two'])
+endfunc
+
+func WithDeferOne()
+  call extend(g:deferred, ['in One'])
+  call writefile(['text'], 'Xfuncdefer')
+  defer delete('Xfuncdefer')
+  defer AddDefer('One')
+  call WithDeferTwo()
+  call extend(g:deferred, ['end One'])
+endfunc
+
+func Test_defer()
+  let g:deferred = []
+  call WithDeferOne()
+
+  call assert_equal(['in One', 'in Two', 'end Two', 'Two2', 'Two1', 'Two0', 'end One', 'One'], g:deferred)
+  unlet g:deferred
+
+  call assert_equal('', glob('Xfuncdefer'))
+endfunc
+
+
 " vim: shiftwidth=2 sts=2 expandtab