vim-patch:8.2.3867: implementation of some list functions too complicated (#24757)

Problem:    Implementation of some list functions too complicated.
Solution:   Refactor do_sort_uniq(), f_count() and extend() (Yegappan
            Lakshmanan, closes vim/vim#9378)

https://github.com/vim/vim/commit/d92813a59877c707e4b64bea6d786aad152acb45

Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>

2 files changed, 368 insertions, 315 deletions

diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
index 044dabb058..b0e0fffe5c 100644
--- a/src/nvim/eval/funcs.c
+++ b/src/nvim/eval/funcs.c
@@ -884,6 +884,86 @@ static void f_copy(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   var_item_copy(NULL, &argvars[0], rettv, false, 0);
 }
 
+/// Count the number of times "needle" occurs in string "haystack".
+///
+/// @param ic  ignore case
+static varnumber_T count_string(const char *haystack, const char *needle, bool ic)
+{
+  varnumber_T n = 0;
+  const char *p = haystack;
+
+  if (p == NULL || needle == NULL || *needle == NUL) {
+    return 0;
+  }
+
+  if (ic) {
+    const size_t len = strlen(needle);
+
+    while (*p != NUL) {
+      if (mb_strnicmp(p, needle, len) == 0) {
+        n++;
+        p += len;
+      } else {
+        MB_PTR_ADV(p);
+      }
+    }
+  } else {
+    const char *next;
+    while ((next = strstr(p, needle)) != NULL) {
+      n++;
+      p = next + strlen(needle);
+    }
+  }
+
+  return n;
+}
+
+/// Count the number of times item "needle" occurs in List "l" starting at index "idx".
+///
+/// @param ic  ignore case
+static varnumber_T count_list(list_T *l, typval_T *needle, int64_t idx, bool ic)
+{
+  if (tv_list_len(l) == 0) {
+    return 0;
+  }
+
+  listitem_T *li = tv_list_find(l, (int)idx);
+  if (li == NULL) {
+    semsg(_(e_list_index_out_of_range_nr), idx);
+    return 0;
+  }
+
+  varnumber_T n = 0;
+
+  for (; li != NULL; li = TV_LIST_ITEM_NEXT(l, li)) {
+    if (tv_equal(TV_LIST_ITEM_TV(li), needle, ic, false)) {
+      n++;
+    }
+  }
+
+  return n;
+}
+
+/// Count the number of times item "needle" occurs in Dict "d".
+///
+/// @param ic  ignore case
+static varnumber_T count_dict(dict_T *d, typval_T *needle, bool ic)
+{
+  if (d == NULL) {
+    return 0;
+  }
+
+  varnumber_T n = 0;
+
+  TV_DICT_ITER(d, di, {
+    if (tv_equal(&di->di_tv, needle, ic, false)) {
+      n++;
+    }
+  });
+
+  return n;
+}
+
 /// "count()" function
 static void f_count(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 {
@@ -895,74 +975,26 @@ static void f_count(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
     ic = (int)tv_get_number_chk(&argvars[2], &error);
   }
 
-  if (argvars[0].v_type == VAR_STRING) {
-    const char *expr = tv_get_string_chk(&argvars[1]);
-    const char *p = argvars[0].vval.v_string;
-
-    if (!error && expr != NULL && *expr != NUL && p != NULL) {
-      if (ic) {
-        const size_t len = strlen(expr);
-
-        while (*p != NUL) {
-          if (mb_strnicmp(p, expr, len) == 0) {
-            n++;
-            p += len;
-          } else {
-            MB_PTR_ADV(p);
-          }
-        }
-      } else {
-        char *next;
-        while ((next = strstr(p, expr)) != NULL) {
-          n++;
-          p = next + strlen(expr);
-        }
-      }
+  if (!error && argvars[0].v_type == VAR_STRING) {
+    n = count_string(argvars[0].vval.v_string, tv_get_string_chk(&argvars[1]), ic);
+  } else if (!error && argvars[0].v_type == VAR_LIST) {
+    int64_t idx = 0;
+    if (argvars[2].v_type != VAR_UNKNOWN
+        && argvars[3].v_type != VAR_UNKNOWN) {
+      idx = (long)tv_get_number_chk(&argvars[3], &error);
     }
-  } else if (argvars[0].v_type == VAR_LIST) {
-    list_T *l = argvars[0].vval.v_list;
-
-    if (l != NULL) {
-      listitem_T *li = tv_list_first(l);
-      if (argvars[2].v_type != VAR_UNKNOWN) {
-        if (argvars[3].v_type != VAR_UNKNOWN) {
-          int64_t idx = tv_get_number_chk(&argvars[3], &error);
-          if (!error) {
-            li = tv_list_find(l, (int)idx);
-            if (li == NULL) {
-              semsg(_(e_list_index_out_of_range_nr), idx);
-            }
-          }
-        }
-        if (error) {
-          li = NULL;
-        }
-      }
-
-      for (; li != NULL; li = TV_LIST_ITEM_NEXT(l, li)) {
-        if (tv_equal(TV_LIST_ITEM_TV(li), &argvars[1], ic, false)) {
-          n++;
-        }
-      }
+    if (!error) {
+      n = count_list(argvars[0].vval.v_list, &argvars[1], idx, ic);
     }
-  } else if (argvars[0].v_type == VAR_DICT) {
+  } else if (!error && argvars[0].v_type == VAR_DICT) {
     dict_T *d = argvars[0].vval.v_dict;
 
     if (d != NULL) {
-      if (argvars[2].v_type != VAR_UNKNOWN) {
-        if (argvars[3].v_type != VAR_UNKNOWN) {
-          emsg(_(e_invarg));
-        }
-      }
-
-      int todo = error ? 0 : (int)d->dv_hashtab.ht_used;
-      for (hashitem_T *hi = d->dv_hashtab.ht_array; todo > 0; hi++) {
-        if (!HASHITEM_EMPTY(hi)) {
-          todo--;
-          if (tv_equal(&TV_DICT_HI2DI(hi)->di_tv, &argvars[1], ic, false)) {
-            n++;
-          }
-        }
+      if (argvars[2].v_type != VAR_UNKNOWN
+          && argvars[3].v_type != VAR_UNKNOWN) {
+        emsg(_(e_invarg));
+      } else {
+        n = count_dict(argvars[0].vval.v_dict, &argvars[1], ic);
       }
     }
   } else {
@@ -1875,104 +1907,124 @@ static void f_flattennew(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   flatten_common(argvars, rettv, true);
 }
 
-/// "extend()" or "extendnew()" function.  "is_new" is true for extendnew().
-static void extend(typval_T *argvars, typval_T *rettv, char *arg_errmsg, bool is_new)
+/// extend() a List. Append List argvars[1] to List argvars[0] before index
+/// argvars[3] and return the resulting list in "rettv".
+///
+/// @param is_new  true for extendnew()
+static void extend_list(typval_T *argvars, const char *arg_errmsg, bool is_new, typval_T *rettv)
 {
-  if (argvars[0].v_type == VAR_LIST && argvars[1].v_type == VAR_LIST) {
-    bool error = false;
+  bool error = false;
 
-    list_T *l1 = argvars[0].vval.v_list;
-    list_T *const l2 = argvars[1].vval.v_list;
-    if (is_new || !value_check_lock(tv_list_locked(l1), arg_errmsg, TV_TRANSLATE)) {
-      if (is_new) {
-        l1 = tv_list_copy(NULL, l1, false, get_copyID());
-        if (l1 == NULL) {
-          return;
-        }
+  list_T *l1 = argvars[0].vval.v_list;
+  list_T *const l2 = argvars[1].vval.v_list;
+  if (is_new || !value_check_lock(tv_list_locked(l1), arg_errmsg, TV_TRANSLATE)) {
+    if (is_new) {
+      l1 = tv_list_copy(NULL, l1, false, get_copyID());
+      if (l1 == NULL) {
+        return;
       }
+    }
 
-      listitem_T *item;
-      if (argvars[2].v_type != VAR_UNKNOWN) {
-        long before = (long)tv_get_number_chk(&argvars[2], &error);
-        if (error) {
-          return;  // Type error; errmsg already given.
-        }
-
-        if (before == tv_list_len(l1)) {
-          item = NULL;
-        } else {
-          item = tv_list_find(l1, (int)before);
-          if (item == NULL) {
-            semsg(_(e_list_index_out_of_range_nr), (int64_t)before);
-            return;
-          }
-        }
-      } else {
-        item = NULL;
+    listitem_T *item;
+    if (argvars[2].v_type != VAR_UNKNOWN) {
+      long before = (long)tv_get_number_chk(&argvars[2], &error);
+      if (error) {
+        return;  // Type error; errmsg already given.
       }
-      tv_list_extend(l1, l2, item);
 
-      if (is_new) {
-        *rettv = (typval_T){
-          .v_type = VAR_LIST,
-          .v_lock = VAR_UNLOCKED,
-          .vval.v_list = l1,
-        };
+      if (before == tv_list_len(l1)) {
+        item = NULL;
       } else {
-        tv_copy(&argvars[0], rettv);
+        item = tv_list_find(l1, (int)before);
+        if (item == NULL) {
+          semsg(_(e_list_index_out_of_range_nr), (int64_t)before);
+          return;
+        }
       }
+    } else {
+      item = NULL;
     }
-  } else if (argvars[0].v_type == VAR_DICT && argvars[1].v_type == VAR_DICT) {
-    dict_T *d1 = argvars[0].vval.v_dict;
-    dict_T *const d2 = argvars[1].vval.v_dict;
-    if (d1 == NULL) {
-      const bool locked = value_check_lock(VAR_FIXED, arg_errmsg, TV_TRANSLATE);
-      (void)locked;
-      assert(locked == true);
-    } else if (d2 == NULL) {
-      // Do nothing
+    tv_list_extend(l1, l2, item);
+
+    if (is_new) {
+      *rettv = (typval_T){
+        .v_type = VAR_LIST,
+        .v_lock = VAR_UNLOCKED,
+        .vval.v_list = l1,
+      };
+    } else {
       tv_copy(&argvars[0], rettv);
-    } else if (is_new || !value_check_lock(d1->dv_lock, arg_errmsg, TV_TRANSLATE)) {
-      if (is_new) {
-        d1 = tv_dict_copy(NULL, d1, false, get_copyID());
-        if (d1 == NULL) {
-          return;
-        }
+    }
+  }
+}
+
+/// extend() a Dict. Append Dict argvars[1] to Dict argvars[0] and return the
+/// resulting Dict in "rettv".
+///
+/// @param is_new  true for extendnew()
+static void extend_dict(typval_T *argvars, const char *arg_errmsg, bool is_new, typval_T *rettv)
+{
+  dict_T *d1 = argvars[0].vval.v_dict;
+  dict_T *const d2 = argvars[1].vval.v_dict;
+  if (d1 == NULL) {
+    const bool locked = value_check_lock(VAR_FIXED, arg_errmsg, TV_TRANSLATE);
+    (void)locked;
+    assert(locked == true);
+  } else if (d2 == NULL) {
+    // Do nothing
+    tv_copy(&argvars[0], rettv);
+  } else if (is_new || !value_check_lock(d1->dv_lock, arg_errmsg, TV_TRANSLATE)) {
+    if (is_new) {
+      d1 = tv_dict_copy(NULL, d1, false, get_copyID());
+      if (d1 == NULL) {
+        return;
       }
+    }
 
-      const char *action = "force";
-      // Check the third argument.
-      if (argvars[2].v_type != VAR_UNKNOWN) {
-        const char *const av[] = { "keep", "force", "error" };
+    const char *action = "force";
+    // Check the third argument.
+    if (argvars[2].v_type != VAR_UNKNOWN) {
+      const char *const av[] = { "keep", "force", "error" };
 
-        action = tv_get_string_chk(&argvars[2]);
-        if (action == NULL) {
-          return;  // Type error; error message already given.
-        }
-        size_t i;
-        for (i = 0; i < ARRAY_SIZE(av); i++) {
-          if (strcmp(action, av[i]) == 0) {
-            break;
-          }
-        }
-        if (i == 3) {
-          semsg(_(e_invarg2), action);
-          return;
+      action = tv_get_string_chk(&argvars[2]);
+      if (action == NULL) {
+        return;  // Type error; error message already given.
+      }
+      size_t i;
+      for (i = 0; i < ARRAY_SIZE(av); i++) {
+        if (strcmp(action, av[i]) == 0) {
+          break;
         }
       }
+      if (i == 3) {
+        semsg(_(e_invarg2), action);
+        return;
+      }
+    }
 
-      tv_dict_extend(d1, d2, action);
+    tv_dict_extend(d1, d2, action);
 
-      if (is_new) {
-        *rettv = (typval_T){
-          .v_type = VAR_DICT,
-          .v_lock = VAR_UNLOCKED,
-          .vval.v_dict = d1,
-        };
-      } else {
-        tv_copy(&argvars[0], rettv);
-      }
+    if (is_new) {
+      *rettv = (typval_T){
+        .v_type = VAR_DICT,
+        .v_lock = VAR_UNLOCKED,
+        .vval.v_dict = d1,
+      };
+    } else {
+      tv_copy(&argvars[0], rettv);
     }
+  }
+}
+
+/// "extend()" or "extendnew()" function.
+///
+/// @param is_new  true for extendnew()
+static void extend(typval_T *argvars, typval_T *rettv, char *arg_errmsg, bool is_new)
+{
+  if (argvars[0].v_type == VAR_LIST && argvars[1].v_type == VAR_LIST) {
+    extend_list(argvars, arg_errmsg, is_new, rettv);
+  } else if (argvars[0].v_type == VAR_DICT && argvars[1].v_type == VAR_DICT) {
+    extend_dict(argvars, arg_errmsg, is_new, rettv);
   } else {
     semsg(_(e_listdictarg), is_new ? "extendnew()" : "extend()");
   }
diff --git a/src/nvim/eval/typval.c b/src/nvim/eval/typval.c
index 5b977e93c9..5f7bd98298 100644
--- a/src/nvim/eval/typval.c
+++ b/src/nvim/eval/typval.c
@@ -36,6 +36,19 @@
 #include "nvim/types.h"
 #include "nvim/vim.h"
 
+/// struct storing information about current sort
+typedef struct {
+  int item_compare_ic;
+  bool item_compare_lc;
+  bool item_compare_numeric;
+  bool item_compare_numbers;
+  bool item_compare_float;
+  const char *item_compare_func;
+  partial_T *item_compare_partial;
+  dict_T *item_compare_selfdict;
+  bool item_compare_func_err;
+} sortinfo_T;
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "eval/typval.c.generated.h"
 #endif
@@ -1065,18 +1078,6 @@ void tv_list_remove(typval_T *argvars, typval_T *rettv, const char *arg_errmsg)
   }
 }
 
-/// struct storing information about current sort
-typedef struct {
-  int item_compare_ic;
-  bool item_compare_lc;
-  bool item_compare_numeric;
-  bool item_compare_numbers;
-  bool item_compare_float;
-  const char *item_compare_func;
-  partial_T *item_compare_partial;
-  dict_T *item_compare_selfdict;
-  bool item_compare_func_err;
-} sortinfo_T;
 static sortinfo_T *sortinfo = NULL;
 
 #define ITEM_COMPARE_FAIL 999
@@ -1252,148 +1253,188 @@ static int item_compare2_not_keeping_zero(const void *s1, const void *s2)
   return item_compare2(s1, s2, false);
 }
 
-/// "sort({list})" function
-static void do_sort_uniq(typval_T *argvars, typval_T *rettv, bool sort)
+/// sort() List "l"
+static void do_sort(list_T *l, sortinfo_T *info)
 {
-  ListSortItem *ptrs;
-  long len;
-  int i;
+  const int len = tv_list_len(l);
 
-  // Pointer to current info struct used in compare function. Save and restore
-  // the current one for nested calls.
-  sortinfo_T info;
-  sortinfo_T *old_sortinfo = sortinfo;
-  sortinfo = &info;
+  // Make an array with each entry pointing to an item in the List.
+  ListSortItem *ptrs = xmalloc((size_t)((unsigned)len * sizeof(ListSortItem)));
 
-  const char *const arg_errmsg = (sort
-                                  ? N_("sort() argument")
-                                  : N_("uniq() argument"));
+  // f_sort(): ptrs will be the list to sort
+  int i = 0;
+  TV_LIST_ITER(l, li, {
+    ptrs[i].item = li;
+    ptrs[i].idx = i;
+    i++;
+  });
 
-  if (argvars[0].v_type != VAR_LIST) {
-    semsg(_(e_listarg), sort ? "sort()" : "uniq()");
-  } else {
-    list_T *const l = argvars[0].vval.v_list;
-    if (value_check_lock(tv_list_locked(l), arg_errmsg, TV_TRANSLATE)) {
-      goto theend;
-    }
-    tv_list_set_ret(rettv, l);
+  info->item_compare_func_err = false;
+  ListSorter item_compare_func = ((info->item_compare_func == NULL
+                                   && info->item_compare_partial == NULL)
+                                  ? item_compare_not_keeping_zero
+                                  : item_compare2_not_keeping_zero);
 
-    len = tv_list_len(l);
-    if (len <= 1) {
-      goto theend;  // short list sorts pretty quickly
+  // Sort the array with item pointers.
+  qsort(ptrs, (size_t)len, sizeof(ListSortItem), item_compare_func);
+  if (!info->item_compare_func_err) {
+    // Clear the list and append the items in the sorted order.
+    l->lv_first = NULL;
+    l->lv_last = NULL;
+    l->lv_idx_item = NULL;
+    l->lv_len = 0;
+    for (i = 0; i < len; i++) {
+      tv_list_append(l, ptrs[i].item);
     }
+  }
+  if (info->item_compare_func_err) {
+    emsg(_("E702: Sort compare function failed"));
+  }
 
-    info.item_compare_ic = false;
-    info.item_compare_lc = false;
-    info.item_compare_numeric = false;
-    info.item_compare_numbers = false;
-    info.item_compare_float = false;
-    info.item_compare_func = NULL;
-    info.item_compare_partial = NULL;
-    info.item_compare_selfdict = NULL;
-
-    if (argvars[1].v_type != VAR_UNKNOWN) {
-      // optional second argument: {func}
-      if (argvars[1].v_type == VAR_FUNC) {
-        info.item_compare_func = argvars[1].vval.v_string;
-      } else if (argvars[1].v_type == VAR_PARTIAL) {
-        info.item_compare_partial = argvars[1].vval.v_partial;
-      } else {
-        bool error = false;
+  xfree(ptrs);
+}
 
-        i = (int)tv_get_number_chk(&argvars[1], &error);
-        if (error) {
-          goto theend;  // type error; errmsg already given
-        }
-        if (i == 1) {
-          info.item_compare_ic = true;
-        } else if (argvars[1].v_type != VAR_NUMBER) {
-          info.item_compare_func = tv_get_string(&argvars[1]);
-        } else if (i != 0) {
-          emsg(_(e_invarg));
-          goto theend;
-        }
-        if (info.item_compare_func != NULL) {
-          if (*info.item_compare_func == NUL) {
-            // empty string means default sort
-            info.item_compare_func = NULL;
-          } else if (strcmp(info.item_compare_func, "n") == 0) {
-            info.item_compare_func = NULL;
-            info.item_compare_numeric = true;
-          } else if (strcmp(info.item_compare_func, "N") == 0) {
-            info.item_compare_func = NULL;
-            info.item_compare_numbers = true;
-          } else if (strcmp(info.item_compare_func, "f") == 0) {
-            info.item_compare_func = NULL;
-            info.item_compare_float = true;
-          } else if (strcmp(info.item_compare_func, "i") == 0) {
-            info.item_compare_func = NULL;
-            info.item_compare_ic = true;
-          } else if (strcmp(info.item_compare_func, "l") == 0) {
-            info.item_compare_func = NULL;
-            info.item_compare_lc = true;
-          }
-        }
-      }
+/// uniq() List "l"
+static void do_uniq(list_T *l, sortinfo_T *info)
+{
+  const int len = tv_list_len(l);
 
-      if (argvars[2].v_type != VAR_UNKNOWN) {
-        // optional third argument: {dict}
-        if (tv_check_for_dict_arg(argvars, 2) == FAIL) {
-          goto theend;
-        }
-        info.item_compare_selfdict = argvars[2].vval.v_dict;
-      }
-    }
+  // Make an array with each entry pointing to an item in the List.
+  ListSortItem *ptrs = xmalloc((size_t)((unsigned)len * sizeof(ListSortItem)));
 
-    // Make an array with each entry pointing to an item in the List.
-    ptrs = xmalloc((size_t)((unsigned)len * sizeof(ListSortItem)));
-
-    if (sort) {
-      info.item_compare_func_err = false;
-      tv_list_item_sort(l, ptrs,
-                        ((info.item_compare_func == NULL
-                          && info.item_compare_partial == NULL)
-                         ? item_compare_not_keeping_zero
-                         : item_compare2_not_keeping_zero),
-                        &info.item_compare_func_err);
-      if (info.item_compare_func_err) {
-        emsg(_("E702: Sort compare function failed"));
-      }
+  // f_uniq(): ptrs will be a stack of items to remove.
+
+  info->item_compare_func_err = false;
+  ListSorter item_compare_func = ((info->item_compare_func == NULL
+                                   && info->item_compare_partial == NULL)
+                                  ? item_compare_keeping_zero
+                                  : item_compare2_keeping_zero);
+
+  for (listitem_T *li = TV_LIST_ITEM_NEXT(l, tv_list_first(l)); li != NULL;) {
+    listitem_T *const prev_li = TV_LIST_ITEM_PREV(l, li);
+    if (item_compare_func(&prev_li, &li) == 0) {
+      li = tv_list_item_remove(l, li);
     } else {
-      ListSorter item_compare_func_ptr;
+      li = TV_LIST_ITEM_NEXT(l, li);
+    }
+    if (info->item_compare_func_err) {
+      emsg(_("E882: Uniq compare function failed"));
+      break;
+    }
+  }
 
-      // f_uniq(): ptrs will be a stack of items to remove.
-      info.item_compare_func_err = false;
-      if (info.item_compare_func != NULL
-          || info.item_compare_partial != NULL) {
-        item_compare_func_ptr = item_compare2_keeping_zero;
-      } else {
-        item_compare_func_ptr = item_compare_keeping_zero;
-      }
+  xfree(ptrs);
+}
 
-      for (listitem_T *li = TV_LIST_ITEM_NEXT(l, tv_list_first(l))
-           ; li != NULL;) {
-        listitem_T *const prev_li = TV_LIST_ITEM_PREV(l, li);
-        if (item_compare_func_ptr(&prev_li, &li) == 0) {
-          li = tv_list_item_remove(l, li);
-        } else {
-          li = TV_LIST_ITEM_NEXT(l, li);
-        }
-        if (info.item_compare_func_err) {
-          emsg(_("E882: Uniq compare function failed"));
-          break;
-        }
+/// Parse the optional arguments to sort() and uniq() and return the values in "info".
+static int parse_sort_uniq_args(typval_T *argvars, sortinfo_T *info)
+{
+  info->item_compare_ic = false;
+  info->item_compare_lc = false;
+  info->item_compare_numeric = false;
+  info->item_compare_numbers = false;
+  info->item_compare_float = false;
+  info->item_compare_func = NULL;
+  info->item_compare_partial = NULL;
+  info->item_compare_selfdict = NULL;
+
+  if (argvars[1].v_type == VAR_UNKNOWN) {
+    return OK;
+  }
+
+  // optional second argument: {func}
+  if (argvars[1].v_type == VAR_FUNC) {
+    info->item_compare_func = argvars[1].vval.v_string;
+  } else if (argvars[1].v_type == VAR_PARTIAL) {
+    info->item_compare_partial = argvars[1].vval.v_partial;
+  } else {
+    bool error = false;
+    int nr = (int)tv_get_number_chk(&argvars[1], &error);
+    if (error) {
+      return FAIL;  // type error; errmsg already given
+    }
+    if (nr == 1) {
+      info->item_compare_ic = true;
+    } else if (argvars[1].v_type != VAR_NUMBER) {
+      info->item_compare_func = tv_get_string(&argvars[1]);
+    } else if (nr != 0) {
+      emsg(_(e_invarg));
+      return FAIL;
+    }
+    if (info->item_compare_func != NULL) {
+      if (*info->item_compare_func == NUL) {
+        // empty string means default sort
+        info->item_compare_func = NULL;
+      } else if (strcmp(info->item_compare_func, "n") == 0) {
+        info->item_compare_func = NULL;
+        info->item_compare_numeric = true;
+      } else if (strcmp(info->item_compare_func, "N") == 0) {
+        info->item_compare_func = NULL;
+        info->item_compare_numbers = true;
+      } else if (strcmp(info->item_compare_func, "f") == 0) {
+        info->item_compare_func = NULL;
+        info->item_compare_float = true;
+      } else if (strcmp(info->item_compare_func, "i") == 0) {
+        info->item_compare_func = NULL;
+        info->item_compare_ic = true;
+      } else if (strcmp(info->item_compare_func, "l") == 0) {
+        info->item_compare_func = NULL;
+        info->item_compare_lc = true;
       }
     }
+  }
+
+  if (argvars[2].v_type != VAR_UNKNOWN) {
+    // optional third argument: {dict}
+    if (tv_check_for_dict_arg(argvars, 2) == FAIL) {
+      return FAIL;
+    }
+    info->item_compare_selfdict = argvars[2].vval.v_dict;
+  }
 
-    xfree(ptrs);
+  return OK;
+}
+
+/// "sort()" or "uniq()" function
+static void do_sort_uniq(typval_T *argvars, typval_T *rettv, bool sort)
+{
+  if (argvars[0].v_type != VAR_LIST) {
+    semsg(_(e_listarg), sort ? "sort()" : "uniq()");
+    return;
+  }
+
+  // Pointer to current info struct used in compare function. Save and restore
+  // the current one for nested calls.
+  sortinfo_T info;
+  sortinfo_T *old_sortinfo = sortinfo;
+  sortinfo = &info;
+
+  const char *const arg_errmsg = (sort ? N_("sort() argument") : N_("uniq() argument"));
+  list_T *const l = argvars[0].vval.v_list;
+  if (value_check_lock(tv_list_locked(l), arg_errmsg, TV_TRANSLATE)) {
+    goto theend;
+  }
+  tv_list_set_ret(rettv, l);
+
+  const int len = tv_list_len(l);
+  if (len <= 1) {
+    goto theend;  // short list sorts pretty quickly
+  }
+  if (parse_sort_uniq_args(argvars, &info) == FAIL) {
+    goto theend;
+  }
+
+  if (sort) {
+    do_sort(l, &info);
+  } else {
+    do_uniq(l, &info);
   }
 
 theend:
   sortinfo = old_sortinfo;
 }
 
-/// "sort"({list})" function
+/// "sort({list})" function
 void f_sort(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 {
   do_sort_uniq(argvars, rettv, true);
@@ -1469,46 +1510,6 @@ void tv_list_reverse(list_T *const l)
   l->lv_idx = l->lv_len - l->lv_idx - 1;
 }
 
-// FIXME Add unit tests for tv_list_item_sort().
-
-/// Sort list using libc qsort
-///
-/// @param[in,out]  l  List to sort, will be sorted in-place.
-/// @param  ptrs  Preallocated array of items to sort, must have at least
-///               tv_list_len(l) entries. Should not be initialized.
-/// @param[in]  item_compare_func  Function used to compare list items.
-/// @param  errp  Location where information about whether error occurred is
-///               saved by item_compare_func. If boolean there appears to be
-///               true list will not be modified. Must be initialized to false
-///               by the caller.
-void tv_list_item_sort(list_T *const l, ListSortItem *const ptrs,
-                       const ListSorter item_compare_func, const bool *errp)
-  FUNC_ATTR_NONNULL_ARG(3, 4)
-{
-  const int len = tv_list_len(l);
-  if (len <= 1) {
-    return;
-  }
-  int i = 0;
-  TV_LIST_ITER(l, li, {
-    ptrs[i].item = li;
-    ptrs[i].idx = i;
-    i++;
-  });
-  // Sort the array with item pointers.
-  qsort(ptrs, (size_t)len, sizeof(ListSortItem), item_compare_func);
-  if (!(*errp)) {
-    // Clear the list and append the items in the sorted order.
-    l->lv_first    = NULL;
-    l->lv_last     = NULL;
-    l->lv_idx_item = NULL;
-    l->lv_len      = 0;
-    for (i = 0; i < len; i++) {
-      tv_list_append(l, ptrs[i].item);
-    }
-  }
-}
-
 //{{{2 Indexing/searching
 
 /// Locate item with a given index in a list and return it