fix: factor out new functions

free_unref_funccal
get_funccal_local_var
get_funccal_args_var
get_current_funccal_dict
set_ref_in_previous_funccal
set_ref_in_call_stack
set_ref_in_func_args

note:
In vim semantic for garbage_collect was changed
(the result of free_unref_funccal is ignored, bug or intentional?)

For nvim I did leave previous behavior thus

did_free = did_free || free_unref_funccal(copyID, testing);
instead of just
free_unref_funccal(copyID, testing);

src/nvim/eval.c

@@ -2813,22 +2813,23 @@ int do_unlet(const char *const name, const size_t name_len, const int forceit)
   hashtab_T *ht = find_var_ht_dict(name, name_len, &varname, &dict);
 
   if (ht != NULL && *varname != NUL) {
-    dict_T *d;
+    dict_T *d = get_current_funccal_dict(ht);
-    if (ht == &globvarht) {
+    if (d == NULL)
-      d = &globvardict;
+	{
-    } else if (current_funccal != NULL
+      if (ht == &globvarht) {
-               && ht == &current_funccal->l_vars.dv_hashtab) {
+        d = &globvardict;
-      d = &current_funccal->l_vars;
+      } else if (ht == &compat_hashtab) {
-    } else if (ht == &compat_hashtab) {
+          d = &vimvardict;
-        d = &vimvardict;
+      } else {
-    } else {
+        dictitem_T *const di = find_var_in_ht(ht, *name, "", 0, false);
-      dictitem_T *const di = find_var_in_ht(ht, *name, "", 0, false);
+        d = di->di_tv.vval.v_dict;
-      d = di->di_tv.vval.v_dict;
+      }
-    }
+      if (d == NULL) {
-    if (d == NULL) {
+        internal_error("do_unlet()");
-      internal_error("do_unlet()");
+        return FAIL;
-      return FAIL;
+      }
     }
+
     hashitem_T *hi = hash_find(ht, (const char_u *)varname);
     if (HASHITEM_EMPTY(hi)) {
       hi = find_hi_in_scoped_ht((const char *)name, &ht);
@@ -4878,11 +4879,7 @@ bool garbage_collect(bool testing)
   // Don't free variables in the previous_funccal list unless they are only
   // referenced through previous_funccal.  This must be first, because if
   // the item is referenced elsewhere the funccal must not be freed.
-  for (funccall_T *fc = previous_funccal; fc != NULL; fc = fc->caller) {
+  ABORTING(set_ref_in_previous_funccal)(copyID);
-    fc->fc_copyID = copyID + 1;
-    ABORTING(set_ref_in_ht)(&fc->l_vars.dv_hashtab, copyID + 1, NULL);
-    ABORTING(set_ref_in_ht)(&fc->l_avars.dv_hashtab, copyID + 1, NULL);
-  }
 
   // script-local variables
   for (int i = 1; i <= ga_scripts.ga_len; ++i) {
@@ -4959,14 +4956,10 @@ bool garbage_collect(bool testing)
   ABORTING(set_ref_in_ht)(&globvarht, copyID, NULL);
 
   // function-local variables
-  for (funccall_T *fc = current_funccal; fc != NULL; fc = fc->caller) {
+  ABORTING(set_ref_in_call_stack)(copyID);
-    fc->fc_copyID = copyID;
-    ABORTING(set_ref_in_ht)(&fc->l_vars.dv_hashtab, copyID, NULL);
-    ABORTING(set_ref_in_ht)(&fc->l_avars.dv_hashtab, copyID, NULL);
-  }
 
   // named functions (matters for closures)
-  ABORTING(set_ref_in_functions(copyID));
+  ABORTING(set_ref_in_functions)(copyID);
 
   // Channels
   {
@@ -4987,10 +4980,7 @@ bool garbage_collect(bool testing)
   }
 
   // function call arguments, if v:testing is set.
-  for (int i = 0; i < funcargs.ga_len; i++) {
+  ABORTING(set_ref_in_func_args)(copyID);
-    ABORTING(set_ref_in_item)(((typval_T **)funcargs.ga_data)[i],
-                              copyID, NULL, NULL);
-  }
 
   // v: vars
   ABORTING(set_ref_in_ht)(&vimvarht, copyID, NULL);
@@ -5033,23 +5023,8 @@ bool garbage_collect(bool testing)
     did_free = free_unref_items(copyID);
 
     // 3. Check if any funccal can be freed now.
-    bool did_free_funccal = false;
+    //    This may call us back recursively.
-    for (funccall_T **pfc = &previous_funccal; *pfc != NULL;) {
+    did_free = did_free || free_unref_funccal(copyID, testing);
-      if (can_free_funccal(*pfc, copyID)) {
-        funccall_T *fc = *pfc;
-        *pfc = fc->caller;
-        free_funccal(fc, true);
-        did_free = true;
-        did_free_funccal = true;
-      } else {
-        pfc = &(*pfc)->caller;
-      }
-    }
-    if (did_free_funccal) {
-      // When a funccal was freed some more items might be garbage
-      // collected, so run again.
-      (void)garbage_collect(testing);
-    }
   } else if (p_verbose > 0) {
     verb_msg(_(
         "Not enough memory to set references, garbage collection aborted!"));
@@ -8526,10 +8501,8 @@ dictitem_T *find_var_in_ht(hashtab_T *const ht,
       case 'b': return (dictitem_T *)&curbuf->b_bufvar;
       case 'w': return (dictitem_T *)&curwin->w_winvar;
       case 't': return (dictitem_T *)&curtab->tp_winvar;
-      case 'l': return (current_funccal == NULL
+      case 'l': return get_funccal_local_var();
-                        ? NULL : (dictitem_T *)&current_funccal->l_vars_var);
+      case 'a': return get_funccal_args_var();
-      case 'a': return (current_funccal == NULL
-                        ? NULL : (dictitem_T *)&get_funccal()->l_avars_var);
     }
     return NULL;
   }

src/nvim/eval/user_funcs.c

@@ -3018,6 +3018,30 @@ int current_func_returned(void)
   return current_funccal->returned;
 }
 
+bool free_unref_funccal(int copyID, int testing)
+{
+  bool did_free = false;
+  bool did_free_funccal = false;
+
+  for (funccall_T **pfc = &previous_funccal; *pfc != NULL;) {
+    if (can_free_funccal(*pfc, copyID)) {
+      funccall_T *fc = *pfc;
+      *pfc = fc->caller;
+      free_funccal(fc, true);
+      did_free = true;
+      did_free_funccal = true;
+    } else {
+      pfc = &(*pfc)->caller;
+    }
+  }
+  if (did_free_funccal) {
+    // When a funccal was freed some more items might be garbage
+    // collected, so run again.
+    (void)garbage_collect(testing);
+  }
+  return did_free;
+}
+
 // Get function call environment based on backtrace debug level
 funccall_T *get_funccal(void)
 {
@@ -3047,6 +3071,16 @@ hashtab_T *get_funccal_local_ht(void)
   return &get_funccal()->l_vars.dv_hashtab;
 }
 
+/// Return the l: scope variable.
+/// Return NULL if there is no current funccal.
+dictitem_T *get_funccal_local_var(void)
+{
+  if (current_funccal == NULL) {
+    return NULL;
+  }
+  return (dictitem_T *)&get_funccal()->l_vars_var;
+}
+
 /// Return the hashtable used for argument in the current funccal.
 /// Return NULL if there is no current funccal.
 hashtab_T *get_funccal_args_ht(void)
@@ -3057,6 +3091,16 @@ hashtab_T *get_funccal_args_ht(void)
   return &get_funccal()->l_avars.dv_hashtab;
 }
 
+/// Return the a: scope variable.
+/// Return NULL if there is no current funccal.
+dictitem_T *get_funccal_args_var(void)
+{
+  if (current_funccal == NULL) {
+    return NULL;
+  }
+  return (dictitem_T *)&current_funccal->l_avars_var;
+}
+
 /*
  * List function variables, if there is a function.
  */
@@ -3068,6 +3112,17 @@ void list_func_vars(int *first)
   }
 }
 
+/// If "ht" is the hashtable for local variables in the current funccal, return
+/// the dict that contains it.
+/// Otherwise return NULL.
+dict_T *get_current_funccal_dict(hashtab_T *ht)
+{
+  if (current_funccal != NULL && ht == &current_funccal->l_vars.dv_hashtab) {
+    return &current_funccal->l_vars;
+  }
+  return NULL;
+}
+
 /// Search hashitem in parent scope.
 hashitem_T *find_hi_in_scoped_ht(const char *name, hashtab_T **pht)
 {
@@ -3134,6 +3189,18 @@ dictitem_T *find_var_in_scoped_ht(const char *name, const size_t namelen,
   return v;
 }
 
+/// Set "copyID + 1" in previous_funccal and callers.
+bool set_ref_in_previous_funccal(int copyID)
+{
+  bool abort = false;
+
+  for (funccall_T *fc = previous_funccal; fc != NULL; fc = fc->caller) {
+    abort = abort || set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID + 1, NULL);
+    abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID + 1, NULL);
+  }
+  return abort;
+}
+
 static bool set_ref_in_funccal(funccall_T *fc, int copyID)
 {
   bool abort = false;
@@ -3147,6 +3214,18 @@ static bool set_ref_in_funccal(funccall_T *fc, int copyID)
   return abort;
 }
 
+/// Set "copyID" in all local vars and arguments in the call stack.
+bool set_ref_in_call_stack(int copyID)
+{
+  bool abort = false;
+
+  for (funccall_T *fc = current_funccal; fc != NULL; fc = fc->caller) {
+    abort = abort || set_ref_in_ht(&fc->l_vars.dv_hashtab, copyID, NULL);
+    abort = abort || set_ref_in_ht(&fc->l_avars.dv_hashtab, copyID, NULL);
+  }
+  return abort;
+}
+
 /// Set "copyID" in all functions available by name.
 bool set_ref_in_functions(int copyID)
 {
@@ -3168,6 +3247,18 @@ bool set_ref_in_functions(int copyID)
   return abort;
 }
  
+/// Set "copyID" in all function arguments.
+bool set_ref_in_func_args(int copyID)
+{
+  bool abort = false;
+
+  for (int i = 0; i < funcargs.ga_len; i++) {
+    abort = abort || set_ref_in_item(((typval_T **)funcargs.ga_data)[i],
+                                     copyID, NULL, NULL);
+  }
+  return abort;
+}
+
 /// Mark all lists and dicts referenced through function "name" with "copyID".
 /// "list_stack" is used to add lists to be marked.  Can be NULL.
 /// "ht_stack" is used to add hashtabs to be marked.  Can be NULL.