eval: Replace some tv_list_item_remove() calls

There is nothing wrong with them, just it is generally better to remove
a range then to remove items individually.

src/nvim/eval.c

@@ -2887,26 +2887,25 @@ static int do_unlet_var(lval_T *const lp, char_u *const name_end, int forceit)
                                   lp->ll_name_len))) {
     return FAIL;
   } else if (lp->ll_range) {
-    listitem_T    *li;
-    listitem_T *ll_li = lp->ll_li;
-    int ll_n1 = lp->ll_n1;
-
-    while (ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= ll_n1)) {
-      li = TV_LIST_ITEM_NEXT(lp->ll_list, ll_li);
-      if (tv_check_lock(TV_LIST_ITEM_TV(ll_li)->v_lock,
+    // Delete a range of List items.
+    listitem_T *const first_li = lp->ll_li;
+    listitem_T *last_li = first_li;
+    for (;;) {
+      listitem_T *const li = TV_LIST_ITEM_NEXT(lp->ll_list, lp->ll_li);
+      if (tv_check_lock(TV_LIST_ITEM_TV(lp->ll_li)->v_lock,
                         (const char *)lp->ll_name,
                         lp->ll_name_len)) {
         return false;
       }
-      ll_li = li;
-      ll_n1++;
-    }
-
-    // Delete a range of List items.
-    while (lp->ll_li != NULL && (lp->ll_empty2 || lp->ll_n2 >= lp->ll_n1)) {
-      lp->ll_li = tv_list_item_remove(lp->ll_list, lp->ll_li);
+      lp->ll_li = li;
       lp->ll_n1++;
+      if (lp->ll_li == NULL || (!lp->ll_empty2 && lp->ll_n2 < lp->ll_n1)) {
+        break;
+      } else {
+        last_li = lp->ll_li;
+      }
     }
+    tv_list_remove_items(lp->ll_list, first_li, last_li);
   } else {
     if (lp->ll_list != NULL) {
       // unlet a List item.
@@ -13123,16 +13122,13 @@ static void f_readfile(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     }
   }   /* while */
 
-  /*
-   * For a negative line count use only the lines at the end of the file,
-   * free the rest.
-   */
-  if (maxline < 0)
-    while (cnt > -maxline) {
-      tv_list_item_remove(rettv->vval.v_list,
-                          tv_list_first(rettv->vval.v_list));
-      cnt--;
-    }
+  // For a negative line count use only the lines at the end of the file,
+  // free the rest.
+  if (maxline < -tv_list_len(rettv->vval.v_list)) {
+    listitem_T *const first_li = tv_list_find(rettv->vval.v_list, maxline);
+    listitem_T *const last_li = tv_list_last(rettv->vval.v_list);
+    tv_list_remove_items(rettv->vval.v_list, first_li, last_li);
+  }
 
   xfree(prev);
   fclose(fd);

src/nvim/eval/typval.c

@@ -284,6 +284,23 @@ void tv_list_drop_items(list_T *const l, listitem_T *const item,
   l->lv_idx_item = NULL;
 }
 
+/// Like tv_list_drop_items, but also frees all removed items
+void tv_list_remove_items(list_T *const l, listitem_T *const item,
+                          listitem_T *const item2)
+  FUNC_ATTR_NONNULL_ALL
+{
+  tv_list_drop_items(l, item, item2);
+  for(listitem_T *li = item;;) {
+    tv_clear(TV_LIST_ITEM_TV(li));
+    listitem_T *const nli = li->li_next;
+    xfree(li);
+    if (li == item2) {
+      break;
+    }
+    li = nli;
+  }
+}
+
 /// Move items "item" to "item2" from list "l" to the end of the list "tgt_l"
 ///
 /// @param[out]  l  List to move from.

test/unit/eval/typval_spec.lua

@@ -444,6 +444,77 @@ describe('typval.c', function()
         alloc_log:check({})
       end)
     end)
+    describe('remove_items()', function()
+      itp('works', function()
+        local l_tv = lua2typvalt({'1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13'})
+        local l = l_tv.vval.v_list
+        local lis = list_items(l)
+        local strings = map(function(li) return li.li_tv.vval.v_string end, lis)
+        -- Three watchers: pointing to first, middle and last elements.
+        local lws = {
+          list_watch(l, lis[1]),
+          list_watch(l, lis[7]),
+          list_watch(l, lis[13]),
+        }
+        alloc_log:clear()
+
+        lib.tv_list_remove_items(l, lis[1], lis[3])
+        eq({'4', '5', '6', '7', '8', '9', '10', '11', '12', '13'}, typvalt2lua(l_tv))
+        eq({lis[4], lis[7], lis[13]}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item})
+        alloc_log:check({
+          a.freed(strings[1]),
+          a.freed(lis[1]),
+          a.freed(strings[2]),
+          a.freed(lis[2]),
+          a.freed(strings[3]),
+          a.freed(lis[3]),
+        })
+
+        lib.tv_list_remove_items(l, lis[11], lis[13])
+        eq({'4', '5', '6', '7', '8', '9', '10'}, typvalt2lua(l_tv))
+        eq({lis[4], lis[7], nil}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item == nil and nil})
+        alloc_log:check({
+          a.freed(strings[11]),
+          a.freed(lis[11]),
+          a.freed(strings[12]),
+          a.freed(lis[12]),
+          a.freed(strings[13]),
+          a.freed(lis[13]),
+        })
+
+        lib.tv_list_remove_items(l, lis[6], lis[8])
+        eq({'4', '5', '9', '10'}, typvalt2lua(l_tv))
+        eq({lis[4], lis[9], nil}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item == nil and nil})
+        alloc_log:check({
+          a.freed(strings[6]),
+          a.freed(lis[6]),
+          a.freed(strings[7]),
+          a.freed(lis[7]),
+          a.freed(strings[8]),
+          a.freed(lis[8]),
+        })
+
+        lib.tv_list_remove_items(l, lis[4], lis[10])
+        eq(empty_list, typvalt2lua(l_tv))
+        eq({true, true, true}, {lws[1].lw_item == nil, lws[2].lw_item == nil, lws[3].lw_item == nil})
+        alloc_log:check({
+          a.freed(strings[4]),
+          a.freed(lis[4]),
+          a.freed(strings[5]),
+          a.freed(lis[5]),
+          a.freed(strings[9]),
+          a.freed(lis[9]),
+          a.freed(strings[10]),
+          a.freed(lis[10]),
+        })
+
+        lib.tv_list_watch_remove(l, lws[1])
+        lib.tv_list_watch_remove(l, lws[2])
+        lib.tv_list_watch_remove(l, lws[3])
+
+        alloc_log:check({})
+      end)
+    end)
     describe('insert', function()
       describe('()', function()
         itp('works', function()