Merge pull request #7762 from ZyX-I/remove-some-listitems

 Remove some tv_list_item_…() functions

3 files changed, 231 insertions, 130 deletions

diff --git a/test/unit/eval/helpers.lua b/test/unit/eval/helpers.lua
index d7399182f7..6babd4be77 100644
--- a/test/unit/eval/helpers.lua
+++ b/test/unit/eval/helpers.lua
@@ -7,7 +7,7 @@ local ffi = helpers.ffi
 local eq = helpers.eq
 
 local eval = cimport('./src/nvim/eval.h', './src/nvim/eval/typval.h',
-                     './src/nvim/hashtab.h')
+                     './src/nvim/hashtab.h', './src/nvim/memory.h')
 
 local null_string = {[true]='NULL string'}
 local null_list = {[true]='NULL list'}
@@ -24,10 +24,19 @@ local nil_value = {[true]='nil'}
 
 local lua2typvalt
 
+local function tv_list_item_alloc()
+  return ffi.cast('listitem_T*', eval.xmalloc(ffi.sizeof('listitem_T')))
+end
+
+local function tv_list_item_free(li)
+  eval.tv_clear(li.li_tv)
+  eval.xfree(li)
+end
+
 local function li_alloc(nogc)
-  local gcfunc = eval.tv_list_item_free
+  local gcfunc = tv_list_item_free
   if nogc then gcfunc = nil end
-  local li = ffi.gc(eval.tv_list_item_alloc(), gcfunc)
+  local li = ffi.gc(tv_list_item_alloc(), gcfunc)
   li.li_next = nil
   li.li_prev = nil
   li.li_tv = {v_type=eval.VAR_UNKNOWN, v_lock=eval.VAR_UNLOCKED}
@@ -41,7 +50,7 @@ local function populate_list(l, lua_l, processed)
   processed[lua_l] = l
   for i = 1, #lua_l do
     local item_tv = ffi.gc(lua2typvalt(lua_l[i], processed), nil)
-    local item_li = eval.tv_list_item_alloc()
+    local item_li = tv_list_item_alloc()
     item_li.li_tv = item_tv
     eval.tv_list_append(l, item_li)
   end
@@ -533,6 +542,7 @@ return {
   typvalt=typvalt,
 
   li_alloc=li_alloc,
+  tv_list_item_free=tv_list_item_free,
 
   dict_iter=dict_iter,
   list_iter=list_iter,
diff --git a/test/unit/eval/typval_spec.lua b/test/unit/eval/typval_spec.lua
index bec74f05fc..b668144175 100644
--- a/test/unit/eval/typval_spec.lua
+++ b/test/unit/eval/typval_spec.lua
@@ -41,6 +41,7 @@ local tbl2callback = eval_helpers.tbl2callback
 local dict_watchers = eval_helpers.dict_watchers
 
 local concat_tables = global_helpers.concat_tables
+local map = global_helpers.map
 
 local lib = cimport('./src/nvim/eval/typval.h', './src/nvim/memory.h',
                     './src/nvim/mbyte.h', './src/nvim/garray.h',
@@ -80,8 +81,6 @@ local function get_alloc_rets(exp_log, res)
   return exp_log
 end
 
-local to_cstr_nofree = function(v) return lib.xstrdup(v) end
-
 local alloc_log = alloc_log_new()
 
 before_each(function()
@@ -121,118 +120,76 @@ end
 describe('typval.c', function()
   describe('list', function()
     describe('item', function()
-      describe('alloc()/free()', function()
+      describe('remove()', function()
         itp('works', function()
-          local li = li_alloc(true)
-          neq(nil, li)
-          lib.tv_list_item_free(li)
-          alloc_log:check({
-            a.li(li),
-            a.freed(li),
-          })
-        end)
-        itp('also frees the value', function()
-          local li
-          local s
-          local l
-          local tv
-          li = li_alloc(true)
-          li.li_tv.v_type = lib.VAR_NUMBER
-          li.li_tv.vval.v_number = 10
-          lib.tv_list_item_free(li)
-          alloc_log:check({
-            a.li(li),
-            a.freed(li),
-          })
-
-          li = li_alloc(true)
-          li.li_tv.v_type = lib.VAR_FLOAT
-          li.li_tv.vval.v_float = 10.5
-          lib.tv_list_item_free(li)
-          alloc_log:check({
-            a.li(li),
-            a.freed(li),
-          })
-
-          li = li_alloc(true)
-          li.li_tv.v_type = lib.VAR_STRING
-          li.li_tv.vval.v_string = nil
-          lib.tv_list_item_free(li)
+          local l = list(1, 2, 3, 4, 5, 6, 7)
+          neq(nil, l)
+          local lis = list_items(l)
           alloc_log:check({
-            a.li(li),
-            a.freed(alloc_log.null),
-            a.freed(li),
+            a.list(l),
+            a.li(lis[1]),
+            a.li(lis[2]),
+            a.li(lis[3]),
+            a.li(lis[4]),
+            a.li(lis[5]),
+            a.li(lis[6]),
+            a.li(lis[7]),
           })
 
-          li = li_alloc(true)
-          li.li_tv.v_type = lib.VAR_STRING
-          s = to_cstr_nofree('test')
-          li.li_tv.vval.v_string = s
-          lib.tv_list_item_free(li)
+          eq(lis[2], lib.tv_list_item_remove(l, lis[1]))
           alloc_log:check({
-            a.li(li),
-            a.str(s, #('test')),
-            a.freed(s),
-            a.freed(li),
+            a.freed(table.remove(lis, 1)),
           })
+          eq(lis, list_items(l))
 
-          li = li_alloc(true)
-          li.li_tv.v_type = lib.VAR_LIST
-          l = ffi.gc(list(), nil)
-          l.lv_refcount = 2
-          li.li_tv.vval.v_list = l
-          lib.tv_list_item_free(li)
+          eq(lis[7], lib.tv_list_item_remove(l, lis[6]))
           alloc_log:check({
-            a.li(li),
-            a.list(l),
-            a.freed(li),
+            a.freed(table.remove(lis)),
           })
-          eq(1, l.lv_refcount)
+          eq(lis, list_items(l))
 
-          li = li_alloc(true)
-          tv = lua2typvalt({})
-          tv.vval.v_dict.dv_refcount = 2
-          li.li_tv = tv
-          lib.tv_list_item_free(li)
+          eq(lis[4], lib.tv_list_item_remove(l, lis[3]))
           alloc_log:check({
-            a.li(li),
-            a.dict(tv.vval.v_dict),
-            a.freed(li),
+            a.freed(table.remove(lis, 3)),
           })
-          eq(1, tv.vval.v_dict.dv_refcount)
+          eq(lis, list_items(l))
         end)
-      end)
-      describe('remove()', function()
-        itp('works', function()
-          local l = list(1, 2, 3, 4, 5, 6, 7)
+        itp('also frees the value', function()
+          local l = list('a', 'b', 'c', 'd')
           neq(nil, l)
           local lis = list_items(l)
           alloc_log:check({
             a.list(l),
+            a.str(lis[1].li_tv.vval.v_string, 1),
             a.li(lis[1]),
+            a.str(lis[2].li_tv.vval.v_string, 1),
             a.li(lis[2]),
+            a.str(lis[3].li_tv.vval.v_string, 1),
             a.li(lis[3]),
+            a.str(lis[4].li_tv.vval.v_string, 1),
             a.li(lis[4]),
-            a.li(lis[5]),
-            a.li(lis[6]),
-            a.li(lis[7]),
           })
+          local strings = map(function(li) return li.li_tv.vval.v_string end,
+                              lis)
 
-          lib.tv_list_item_remove(l, lis[1])
+          eq(lis[2], lib.tv_list_item_remove(l, lis[1]))
           alloc_log:check({
+            a.freed(table.remove(strings, 1)),
             a.freed(table.remove(lis, 1)),
           })
           eq(lis, list_items(l))
 
-          lib.tv_list_item_remove(l, lis[6])
+          eq(lis[3], lib.tv_list_item_remove(l, lis[2]))
           alloc_log:check({
-            a.freed(table.remove(lis)),
+            a.freed(table.remove(strings, 2)),
+            a.freed(table.remove(lis, 2)),
           })
           eq(lis, list_items(l))
 
-          lib.tv_list_item_remove(l, lis[3])
+          eq(nil, lib.tv_list_item_remove(l, lis[2]))
           alloc_log:check({
-            a.freed(table.remove(lis, 3)),
+            a.freed(table.remove(strings, 2)),
+            a.freed(table.remove(lis, 2)),
           })
           eq(lis, list_items(l))
         end)
@@ -257,19 +214,19 @@ describe('typval.c', function()
             a.li(lis[7]),
           })
 
-          lib.tv_list_item_remove(l, lis[4])
+          eq(lis[5], lib.tv_list_item_remove(l, lis[4]))
           alloc_log:check({a.freed(lis[4])})
           eq({lis[1], lis[5], lis[7]}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item})
 
-          lib.tv_list_item_remove(l, lis[2])
+          eq(lis[3], lib.tv_list_item_remove(l, lis[2]))
           alloc_log:check({a.freed(lis[2])})
           eq({lis[1], lis[5], lis[7]}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item})
 
-          lib.tv_list_item_remove(l, lis[7])
+          eq(nil, lib.tv_list_item_remove(l, lis[7]))
           alloc_log:check({a.freed(lis[7])})
           eq({lis[1], lis[5], nil}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item == nil and nil})
 
-          lib.tv_list_item_remove(l, lis[1])
+          eq(lis[3], lib.tv_list_item_remove(l, lis[1]))
           alloc_log:check({a.freed(lis[1])})
           eq({lis[3], lis[5], nil}, {lws[1].lw_item, lws[2].lw_item, lws[3].lw_item == nil and nil})
 
@@ -449,7 +406,7 @@ describe('typval.c', function()
         })
       end)
     end)
-    describe('remove_items()', function()
+    describe('drop_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
@@ -462,21 +419,92 @@ describe('typval.c', function()
         }
         alloc_log:clear()
 
-        lib.tv_list_remove_items(l, lis[1], lis[3])
+        lib.tv_list_drop_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})
 
-        lib.tv_list_remove_items(l, lis[11], lis[13])
+        lib.tv_list_drop_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})
 
-        lib.tv_list_remove_items(l, lis[6], lis[8])
+        lib.tv_list_drop_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})
 
+        lib.tv_list_drop_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})
+
+        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('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])
@@ -678,6 +706,66 @@ describe('typval.c', function()
           eq({int(-100500), int(100500)}, typvalt2lua(l_tv))
         end)
       end)
+      describe('tv()', function()
+        itp('works', function()
+          local l_tv = lua2typvalt(empty_list)
+          local l = l_tv.vval.v_list
+
+          local l_l_tv = lua2typvalt(empty_list)
+          alloc_log:clear()
+          local l_l = l_l_tv.vval.v_list
+          eq(1, l_l.lv_refcount)
+          lib.tv_list_append_tv(l, l_l_tv)
+          eq(2, l_l.lv_refcount)
+          eq(l_l, l.lv_first.li_tv.vval.v_list)
+          alloc_log:check({
+            a.li(l.lv_first),
+          })
+
+          local l_s_tv = lua2typvalt('test')
+          alloc_log:check({
+            a.str(l_s_tv.vval.v_string, 'test'),
+          })
+          lib.tv_list_append_tv(l, l_s_tv)
+          alloc_log:check({
+            a.li(l.lv_last),
+            a.str(l.lv_last.li_tv.vval.v_string, 'test'),
+          })
+
+          eq({empty_list, 'test'}, typvalt2lua(l_tv))
+        end)
+      end)
+      describe('owned tv()', function()
+        itp('works', function()
+          local l_tv = lua2typvalt(empty_list)
+          local l = l_tv.vval.v_list
+
+          local l_l_tv = lua2typvalt(empty_list)
+          alloc_log:clear()
+          local l_l = l_l_tv.vval.v_list
+          eq(1, l_l.lv_refcount)
+          lib.tv_list_append_owned_tv(l, l_l_tv)
+          eq(1, l_l.lv_refcount)
+          l_l.lv_refcount = l_l.lv_refcount + 1
+          eq(l_l, l.lv_first.li_tv.vval.v_list)
+          alloc_log:check({
+            a.li(l.lv_first),
+          })
+
+          local l_s_tv = ffi.gc(lua2typvalt('test'), nil)
+          alloc_log:check({
+            a.str(l_s_tv.vval.v_string, 'test'),
+          })
+          lib.tv_list_append_owned_tv(l, l_s_tv)
+          eq(l_s_tv.vval.v_string, l.lv_last.li_tv.vval.v_string)
+          l_s_tv.vval.v_string = nil
+          alloc_log:check({
+            a.li(l.lv_last),
+          })
+
+          eq({empty_list, 'test'}, typvalt2lua(l_tv))
+        end)
+      end)
     end)
     describe('copy()', function()
       local function tv_list_copy(...)
diff --git a/test/unit/helpers.lua b/test/unit/helpers.lua
index b1e709c444..87c838dece 100644
--- a/test/unit/helpers.lua
+++ b/test/unit/helpers.lua
@@ -650,8 +650,6 @@ local function itp_child(wr, func)
     collectgarbage('stop')
     child_sethook(wr)
     err, emsg = pcall(func)
-    collectgarbage('restart')
-    collectgarbage()
     debug.sethook()
   end
   emsg = tostring(emsg)
@@ -662,14 +660,15 @@ local function itp_child(wr, func)
     end
     sc.write(wr, ('-\n%05u\n%s'):format(#emsg, emsg))
     deinit()
-    sc.close(wr)
-    sc.exit(1)
   else
     sc.write(wr, '+\n')
     deinit()
-    sc.close(wr)
-    sc.exit(0)
   end
+  collectgarbage('restart')
+  collectgarbage()
+  sc.write(wr, '$\n')
+  sc.close(wr)
+  sc.exit(err and 0 or 1)
 end
 
 local function check_child_err(rd)
@@ -690,44 +689,48 @@ local function check_child_err(rd)
       break
     end
     trace[#trace + 1] = traceline
-    table.remove(trace, maxtrace + 1)
+    if #trace > maxtrace then
+      table.remove(trace, 1)
+    end
   end
   local res = sc.read(rd, 2)
-  if #res ~= 2 then
-    local error
-    if #trace == 0 then
-      error = '\nTest crashed, no trace available\n'
-    else
-      error = '\nTest crashed, trace:\n' .. tracehelp
-      for i = 1, #trace do
-        error = error .. trace[i]
+  if #res == 2 then
+    local err = ''
+    if res ~= '+\n' then
+      eq('-\n', res)
+      local len_s = sc.read(rd, 5)
+      local len = tonumber(len_s)
+      neq(0, len)
+      if os.getenv('NVIM_TEST_TRACE_ON_ERROR') == '1' and #trace ~= 0 then
+        err = '\nTest failed, trace:\n' .. tracehelp
+        for _, traceline in ipairs(trace) do
+          err = err .. traceline
+        end
       end
+      err = err .. sc.read(rd, len + 1)
     end
-    if not did_traceline then
-      error = error .. '\nNo end of trace occurred'
-    end
-    local cc_err, cc_emsg = pcall(check_cores, Paths.test_luajit_prg, true)
-    if not cc_err then
-      error = error .. '\ncheck_cores failed: ' .. cc_emsg
+    local eres = sc.read(rd, 2)
+    if eres ~= '$\n' then
+      if #trace == 0 then
+        err = '\nTest crashed, no trace available\n'
+      else
+        err = '\nTest crashed, trace:\n' .. tracehelp
+        for i = 1, #trace do
+          err = err .. trace[i]
+        end
+      end
+      if not did_traceline then
+        err = err .. '\nNo end of trace occurred'
+      end
+      local cc_err, cc_emsg = pcall(check_cores, Paths.test_luajit_prg, true)
+      if not cc_err then
+        err = err .. '\ncheck_cores failed: ' .. cc_emsg
+      end
     end
-    assert.just_fail(error)
-  end
-  if res == '+\n' then
-    return
-  end
-  eq('-\n', res)
-  local len_s = sc.read(rd, 5)
-  local len = tonumber(len_s)
-  neq(0, len)
-  local err = ''
-  if os.getenv('NVIM_TEST_TRACE_ON_ERROR') == '1' and #trace ~= 0 then
-    err = '\nTest failed, trace:\n' .. tracehelp
-    for _, traceline in ipairs(trace) do
-      err = err .. traceline
+    if err ~= '' then
+      assert.just_fail(err)
     end
   end
-  err = err .. sc.read(rd, len + 1)
-  assert.just_fail(err)
 end
 
 local function itp_parent(rd, pid, allow_failure)