2019-11-15 10:21:45 -07:00
|
|
|
local helpers = require("test.unit.helpers")(after_each)
|
|
|
|
local itp = helpers.gen_itp(it)
|
|
|
|
|
|
|
|
local ffi = helpers.ffi
|
|
|
|
local eq = helpers.eq
|
|
|
|
local ok = helpers.ok
|
|
|
|
|
|
|
|
local lib = helpers.cimport("./src/nvim/marktree.h")
|
|
|
|
|
|
|
|
local function tablelength(t)
|
|
|
|
local count = 0
|
|
|
|
for _ in pairs(t) do count = count + 1 end
|
|
|
|
return count
|
|
|
|
end
|
|
|
|
|
|
|
|
local function pos_leq(a, b)
|
|
|
|
return a[1] < b[1] or (a[1] == b[1] and a[2] <= b[2])
|
|
|
|
end
|
|
|
|
|
|
|
|
-- Checks that shadow and tree is consistent, and optionally
|
|
|
|
-- return the order
|
|
|
|
local function shadoworder(tree, shadow, iter, giveorder)
|
|
|
|
ok(iter ~= nil)
|
|
|
|
local status = lib.marktree_itr_first(tree, iter)
|
|
|
|
local count = 0
|
|
|
|
local pos2id, id2pos = {}, {}
|
|
|
|
local last
|
|
|
|
if not status and next(shadow) == nil then
|
|
|
|
return pos2id, id2pos
|
|
|
|
end
|
|
|
|
repeat
|
|
|
|
local mark = lib.marktree_itr_current(iter)
|
|
|
|
local id = tonumber(mark.id)
|
|
|
|
local spos = shadow[id]
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
if (mark.pos.row ~= spos[1] or mark.pos.col ~= spos[2]) then
|
|
|
|
error("invalid pos for "..id..":("..mark.pos.row..", "..mark.pos.col..") instead of ("..spos[1]..", "..spos[2]..")")
|
2019-11-15 10:21:45 -07:00
|
|
|
end
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
if lib.mt_right_test(mark) ~= spos[3] then
|
|
|
|
error("invalid gravity for "..id..":("..mark.pos.row..", "..mark.pos.col..")")
|
2019-11-15 10:21:45 -07:00
|
|
|
end
|
|
|
|
if count > 0 then
|
|
|
|
if not pos_leq(last, spos) then
|
|
|
|
error("DISORDER")
|
|
|
|
end
|
|
|
|
end
|
|
|
|
count = count + 1
|
|
|
|
last = spos
|
|
|
|
if giveorder then
|
|
|
|
pos2id[count] = id
|
|
|
|
id2pos[id] = count
|
|
|
|
end
|
|
|
|
until not lib.marktree_itr_next(tree, iter)
|
|
|
|
local shadowlen = tablelength(shadow)
|
|
|
|
if shadowlen ~= count then
|
|
|
|
error("missed some keys? (shadow "..shadowlen..", tree "..count..")")
|
|
|
|
end
|
|
|
|
return id2pos, pos2id
|
|
|
|
end
|
|
|
|
|
|
|
|
local function shadowsplice(shadow, start, old_extent, new_extent)
|
|
|
|
local old_end = {start[1] + old_extent[1],
|
|
|
|
(old_extent[1] == 0 and start[2] or 0) + old_extent[2]}
|
|
|
|
local new_end = {start[1] + new_extent[1],
|
|
|
|
(new_extent[1] == 0 and start[2] or 0) + new_extent[2]}
|
|
|
|
local delta = {new_end[1] - old_end[1], new_end[2] - old_end[2]}
|
|
|
|
for _, pos in pairs(shadow) do
|
|
|
|
if pos_leq(start, pos) then
|
|
|
|
if pos_leq(pos, old_end) then
|
|
|
|
-- delete region
|
|
|
|
if pos[3] then -- right gravity
|
|
|
|
pos[1], pos[2] = new_end[1], new_end[2]
|
|
|
|
else
|
|
|
|
pos[1], pos[2] = start[1], start[2]
|
|
|
|
end
|
|
|
|
else
|
|
|
|
if pos[1] == old_end[1] then
|
|
|
|
pos[2] = pos[2] + delta[2]
|
|
|
|
end
|
|
|
|
pos[1] = pos[1] + delta[1]
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
local function dosplice(tree, shadow, start, old_extent, new_extent)
|
|
|
|
lib.marktree_splice(tree, start[1], start[2], old_extent[1], old_extent[2], new_extent[1], new_extent[2])
|
|
|
|
shadowsplice(shadow, start, old_extent, new_extent)
|
|
|
|
end
|
|
|
|
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
local last_id = nil
|
|
|
|
|
|
|
|
local function put(tree, row, col, gravitate)
|
|
|
|
last_id = last_id + 1
|
|
|
|
local my_id = last_id
|
|
|
|
|
|
|
|
lib.marktree_put_test(tree, my_id, row, col, gravitate);
|
|
|
|
return my_id
|
|
|
|
end
|
|
|
|
|
2019-11-15 10:21:45 -07:00
|
|
|
describe('marktree', function()
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
before_each(function()
|
|
|
|
last_id = 0
|
|
|
|
end)
|
|
|
|
|
2019-11-15 10:21:45 -07:00
|
|
|
itp('works', function()
|
|
|
|
local tree = ffi.new("MarkTree[1]") -- zero initialized by luajit
|
|
|
|
local shadow = {}
|
|
|
|
local iter = ffi.new("MarkTreeIter[1]")
|
|
|
|
local iter2 = ffi.new("MarkTreeIter[1]")
|
|
|
|
|
|
|
|
for i = 1,100 do
|
|
|
|
for j = 1,100 do
|
|
|
|
local gravitate = (i%2) > 0
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
local id = put(tree, j, i, gravitate)
|
2019-11-15 10:21:45 -07:00
|
|
|
ok(id > 0)
|
|
|
|
eq(nil, shadow[id])
|
|
|
|
shadow[id] = {j,i,gravitate}
|
|
|
|
end
|
|
|
|
-- checking every insert is too slow, but this is ok
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
end
|
|
|
|
|
|
|
|
-- ss = lib.mt_inspect_rec(tree)
|
|
|
|
-- io.stdout:write(ffi.string(ss))
|
|
|
|
-- io.stdout:flush()
|
|
|
|
|
|
|
|
local id2pos, pos2id = shadoworder(tree, shadow, iter)
|
|
|
|
eq({}, pos2id) -- not set if not requested
|
|
|
|
eq({}, id2pos)
|
|
|
|
|
|
|
|
for i,ipos in pairs(shadow) do
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
local p = lib.marktree_lookup_ns(tree, -1, i, false, iter)
|
|
|
|
eq(ipos[1], p.pos.row)
|
|
|
|
eq(ipos[2], p.pos.col)
|
2019-11-15 10:21:45 -07:00
|
|
|
local k = lib.marktree_itr_current(iter)
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
eq(ipos[1], k.pos.row)
|
|
|
|
eq(ipos[2], k.pos.col, ipos[1])
|
2019-11-15 10:21:45 -07:00
|
|
|
lib.marktree_itr_next(tree, iter)
|
|
|
|
-- TODO(bfredl): use id2pos to check neighbour?
|
|
|
|
-- local k2 = lib.marktree_itr_current(iter)
|
|
|
|
end
|
|
|
|
|
|
|
|
for i,ipos in pairs(shadow) do
|
|
|
|
lib.marktree_itr_get(tree, ipos[1], ipos[2], iter)
|
|
|
|
local k = lib.marktree_itr_current(iter)
|
|
|
|
eq(i, tonumber(k.id))
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
eq(ipos[1], k.pos.row)
|
|
|
|
eq(ipos[2], k.pos.col)
|
2019-11-15 10:21:45 -07:00
|
|
|
end
|
|
|
|
|
|
|
|
ok(lib.marktree_itr_first(tree, iter))
|
|
|
|
local del = lib.marktree_itr_current(iter)
|
|
|
|
|
|
|
|
lib.marktree_del_itr(tree, iter, false)
|
|
|
|
shadow[tonumber(del.id)] = nil
|
|
|
|
shadoworder(tree, shadow, iter)
|
|
|
|
|
|
|
|
for _, ci in ipairs({0,-1,1,-2,2,-10,10}) do
|
|
|
|
for i = 1,100 do
|
|
|
|
lib.marktree_itr_get(tree, i, 50+ci, iter)
|
|
|
|
local k = lib.marktree_itr_current(iter)
|
|
|
|
local id = tonumber(k.id)
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
eq(shadow[id][1], k.pos.row)
|
|
|
|
eq(shadow[id][2], k.pos.col)
|
2019-11-15 10:21:45 -07:00
|
|
|
lib.marktree_del_itr(tree, iter, false)
|
|
|
|
shadow[id] = nil
|
|
|
|
end
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
shadoworder(tree, shadow, iter)
|
|
|
|
end
|
|
|
|
|
|
|
|
-- NB: this is quite rudimentary. We rely on
|
|
|
|
-- functional tests exercising splicing quite a bit
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
dosplice(tree, shadow, {2,2}, {0,5}, {1, 2})
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
shadoworder(tree, shadow, iter)
|
|
|
|
dosplice(tree, shadow, {30,2}, {30,5}, {1, 2})
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
shadoworder(tree, shadow, iter)
|
|
|
|
|
|
|
|
dosplice(tree, shadow, {5,3}, {0,2}, {0, 5})
|
|
|
|
shadoworder(tree, shadow, iter)
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
|
|
|
|
-- build then burn (HOORAY! HOORAY!)
|
|
|
|
while next(shadow) do
|
|
|
|
lib.marktree_itr_first(tree, iter)
|
|
|
|
-- delete every other key for fun and profit
|
|
|
|
while true do
|
|
|
|
local k = lib.marktree_itr_current(iter)
|
|
|
|
lib.marktree_del_itr(tree, iter, false)
|
|
|
|
ok(shadow[tonumber(k.id)] ~= nil)
|
|
|
|
shadow[tonumber(k.id)] = nil
|
|
|
|
local stat = lib.marktree_itr_next(tree, iter)
|
|
|
|
if not stat then
|
|
|
|
break
|
|
|
|
end
|
|
|
|
end
|
|
|
|
lib.marktree_check(tree)
|
|
|
|
shadoworder(tree, shadow, iter2)
|
|
|
|
end
|
extmark: fix deletable nodes in MarkTree sometimes getting skipped
As per #14236, performing extmark cleanup in a certain namespace does
not guarantee removing all the extmarks inside given namespace.
The issue resides within the tree node removal method and results in
a couple of rare edge cases.
To demonstrate what causes this bug, I'll give an example covering one
of the edge cases.
=== AN EXAMPLE ===
(A) (B) (C) (D) (E)
--------- --------- --------- --------- ---------
<0, 1> <0, 1> <0, 1> <0, 1> <0, 1>
<0, 2> <0, 2> <0, 2> <0, 2> <0, 2>
<0, 3> <0, 3> <0, 3> <0, 3> <0, 3>
<0, 4> <0, 4> <0, 4> <0, 4> <0, 4>
<0, 5> <0, 5> <0, 5> <0, 5> <0, 5>
<0, 6> <0, 6> <0, 6> <0, 6> <0, 6>
<0, 7> <0, 7> <0, 7> <0, 7> <0, 7>
<0, 8> <0, 8> <0, 8> <0, 8> <0, 8>
<0, 9> <0, 9> * * <0, 9> * <0, 9>
[0, 10] * [0, 10] <0, 9> [0, 11] [0, 11]
[0, 11] [0, 11] [0, 11] [0, 12] [0, 12] *
[0, 12] [0, 12] [0, 12] [0, 13] [0, 13]
[0, 13] [0, 13] [0, 13] [0, 14] [0, 14]
[0, 14] [0, 14] [0, 14] [0, 15] [0, 15]
[0, 15] [0, 15] [0, 15] [0, 16] [0, 16]
[0, 16] [0, 16] [0, 16] [0, 17] [0, 17]
[0, 17] [0, 17] [0, 17] [0, 18] [0, 18]
[0, 18] [0, 18] [0, 18] [0, 19] [0, 19]
[0, 19] [0, 19] [0, 19] [0, 20] [0, 20]
[0, 20] [0, 20] [0, 20]
DIAGRAM EXPLANATION
* Every column is a state of the marktree at a certain stage.
* To make it simple, I don't draw the whole tree. What you see are
2 leftmost parent nodes ([0, 10], [0, 20]) and their children placed
in order `MarkTreeIter` would iterate through. From top to bottom.
* Numbers on this diagram represent extmark coordinates. Relative
positioning and actual mark IDs used by the marktree are avoided
for simplicity.
* 2 types of brackets around coordinates represent 2 different
extmark namespaces (`ns_id`s).
* '*' shows iterator position.
ACTUAL EXPLANATION
Let's assume, we have two sets of extmarks from 2 different plugins:
* Plugin1: <0, 1-9>
* Plugin2: [0, 10-20]
1. Plugin2 calls
`vim.api.nvim_buf_clear_namespace(buf_handle, ns_id, 0, -1)`
to clear all its extmarks which results in `extmark_clear` call.
2. The iteration process goes on ignoring extmarks with irrelevant
`ns_id` from Plugin1, until it reaches [0, 10], entering state (A).
3. At the end of cleaning up process, `marktree_del_itr` gets called.
This function is supposed to remove given node and, if necessary,
restructure the tree. Also, move the iterator to the next node.
The bug occurs in this function.
4. The iterator goes backwards to the node's last child, to put it
in the place of its deleted parent later. (B)
5. The parent node is deleted and replaced with its child node. (C)
6. Since now this node has 8 children, which is less than
`MT_BRANCH_FACTOR - 1`, it get's merged with the next node. (D)
7. Finally, since at (B) the iterator went backward, it goes forward
twice, skipping [0, 11] node, causing this extmark to persist,
causing the bug. (E)
ANALYSIS AND SOLUTION
The algorithm works perfectly when the parent node gets replaced by
its child, but no merging occurs. I.e. the exact same diagram,
but without the (D) stage. If not for (D), it would iterate to <0, 9>
and then to [0, 11]. So, iterating twice makes sense. The actual problem
is in (C) stage, because the iterator index isn't adjusted and still
pointing to no longer existent node. So my solution is to adjust
iterator index after removing the child node.
More info: https://github.com/neovim/neovim/pull/14719
2021-06-04 03:38:13 -07:00
|
|
|
|
|
|
|
-- Check iterator validity for 2 specific edge cases:
|
|
|
|
-- https://github.com/neovim/neovim/pull/14719
|
|
|
|
lib.marktree_clear(tree)
|
|
|
|
for i = 1,20 do
|
refactor(extmarks): use a more efficient representation
marktree.c was originally constructed as a "generic" datatype,
to make the prototyping of its internal logic as simple as possible
and also as the usecases for various kinds of extmarks/decorations was not yet decided.
As a consequence of this, various extra indirections and allocations was
needed to use marktree to implement extmarks (ns/id pairs) and
decorations of different kinds (some which is just a single highlight
id, other an allocated list of virtual text/lines)
This change removes a lot of indirection, by making Marktree specialized
for the usecase. In particular, the namespace id and mark id is stored
directly, instead of the 64-bit global id particular to the Marktree
struct. This removes the two maps needed to convert between global and
per-ns ids.
Also, "small" decorations are stored inline, i.e. those who
doesn't refer to external heap memory anyway. That is highlights (with
priority+flags) are stored inline, while virtual text, which anyway
occurs a lot of heap allocations, do not. (previously a hack was used
to elide heap allocations for highlights with standard prio+flags)
TODO(bfredl): the functionaltest-lua CI version of gcc is having
severe issues with uint16_t bitfields, so splitting up compound
assignments and redundant casts are needed. Clean this up once we switch
to a working compiler version.
2021-10-25 12:51:29 -07:00
|
|
|
put(tree, i, i, false)
|
extmark: fix deletable nodes in MarkTree sometimes getting skipped
As per #14236, performing extmark cleanup in a certain namespace does
not guarantee removing all the extmarks inside given namespace.
The issue resides within the tree node removal method and results in
a couple of rare edge cases.
To demonstrate what causes this bug, I'll give an example covering one
of the edge cases.
=== AN EXAMPLE ===
(A) (B) (C) (D) (E)
--------- --------- --------- --------- ---------
<0, 1> <0, 1> <0, 1> <0, 1> <0, 1>
<0, 2> <0, 2> <0, 2> <0, 2> <0, 2>
<0, 3> <0, 3> <0, 3> <0, 3> <0, 3>
<0, 4> <0, 4> <0, 4> <0, 4> <0, 4>
<0, 5> <0, 5> <0, 5> <0, 5> <0, 5>
<0, 6> <0, 6> <0, 6> <0, 6> <0, 6>
<0, 7> <0, 7> <0, 7> <0, 7> <0, 7>
<0, 8> <0, 8> <0, 8> <0, 8> <0, 8>
<0, 9> <0, 9> * * <0, 9> * <0, 9>
[0, 10] * [0, 10] <0, 9> [0, 11] [0, 11]
[0, 11] [0, 11] [0, 11] [0, 12] [0, 12] *
[0, 12] [0, 12] [0, 12] [0, 13] [0, 13]
[0, 13] [0, 13] [0, 13] [0, 14] [0, 14]
[0, 14] [0, 14] [0, 14] [0, 15] [0, 15]
[0, 15] [0, 15] [0, 15] [0, 16] [0, 16]
[0, 16] [0, 16] [0, 16] [0, 17] [0, 17]
[0, 17] [0, 17] [0, 17] [0, 18] [0, 18]
[0, 18] [0, 18] [0, 18] [0, 19] [0, 19]
[0, 19] [0, 19] [0, 19] [0, 20] [0, 20]
[0, 20] [0, 20] [0, 20]
DIAGRAM EXPLANATION
* Every column is a state of the marktree at a certain stage.
* To make it simple, I don't draw the whole tree. What you see are
2 leftmost parent nodes ([0, 10], [0, 20]) and their children placed
in order `MarkTreeIter` would iterate through. From top to bottom.
* Numbers on this diagram represent extmark coordinates. Relative
positioning and actual mark IDs used by the marktree are avoided
for simplicity.
* 2 types of brackets around coordinates represent 2 different
extmark namespaces (`ns_id`s).
* '*' shows iterator position.
ACTUAL EXPLANATION
Let's assume, we have two sets of extmarks from 2 different plugins:
* Plugin1: <0, 1-9>
* Plugin2: [0, 10-20]
1. Plugin2 calls
`vim.api.nvim_buf_clear_namespace(buf_handle, ns_id, 0, -1)`
to clear all its extmarks which results in `extmark_clear` call.
2. The iteration process goes on ignoring extmarks with irrelevant
`ns_id` from Plugin1, until it reaches [0, 10], entering state (A).
3. At the end of cleaning up process, `marktree_del_itr` gets called.
This function is supposed to remove given node and, if necessary,
restructure the tree. Also, move the iterator to the next node.
The bug occurs in this function.
4. The iterator goes backwards to the node's last child, to put it
in the place of its deleted parent later. (B)
5. The parent node is deleted and replaced with its child node. (C)
6. Since now this node has 8 children, which is less than
`MT_BRANCH_FACTOR - 1`, it get's merged with the next node. (D)
7. Finally, since at (B) the iterator went backward, it goes forward
twice, skipping [0, 11] node, causing this extmark to persist,
causing the bug. (E)
ANALYSIS AND SOLUTION
The algorithm works perfectly when the parent node gets replaced by
its child, but no merging occurs. I.e. the exact same diagram,
but without the (D) stage. If not for (D), it would iterate to <0, 9>
and then to [0, 11]. So, iterating twice makes sense. The actual problem
is in (C) stage, because the iterator index isn't adjusted and still
pointing to no longer existent node. So my solution is to adjust
iterator index after removing the child node.
More info: https://github.com/neovim/neovim/pull/14719
2021-06-04 03:38:13 -07:00
|
|
|
end
|
|
|
|
|
|
|
|
lib.marktree_itr_get(tree, 10, 10, iter)
|
|
|
|
lib.marktree_del_itr(tree, iter, false)
|
|
|
|
eq(11, iter[0].node.key[iter[0].i].pos.col)
|
|
|
|
|
|
|
|
lib.marktree_itr_get(tree, 11, 11, iter)
|
|
|
|
lib.marktree_del_itr(tree, iter, false)
|
|
|
|
eq(12, iter[0].node.key[iter[0].i].pos.col)
|
2019-11-15 10:21:45 -07:00
|
|
|
end)
|
|
|
|
end)
|