diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b5d9d5..6cdfc66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.4.3 + +Fixed a bug where `split` would sometimes produce invalid trees. + ## v1.4.0 The performance of `concat`, `split`, `drop`, `take`, `insert` and `delete` have been improved. diff --git a/gleam.toml b/gleam.toml index 71b8e89..97c6949 100644 --- a/gleam.toml +++ b/gleam.toml @@ -1,5 +1,5 @@ name = "iv" -version = "1.4.2" +version = "1.4.3" # Fill out these fields if you intend to generate HTML documentation or publish # your project to the Hex package manager. diff --git a/src/iv/internal/node.gleam b/src/iv/internal/node.gleam index 7e87ff7..74b3708 100644 --- a/src/iv/internal/node.gleam +++ b/src/iv/internal/node.gleam @@ -573,7 +573,9 @@ fn sum_children_counts(count, node) { } @external(erlang, "iv_ffi", "sum_node_children_counts_skip_first") -fn sum_node_children_counts_skip_first(nodes: vector.Vector(Node(item))) -> Int { +fn sum_node_children_counts_skip_first( + nodes: vector.Vector(Node(item)), +) -> Int { vector.fold_skip_first(nodes, 0, sum_children_counts) } @@ -870,7 +872,10 @@ fn direct_append_unbalanced( case direct_concat(left_shift - branch_bits, left_last, right_shift, right) { Concatenated(updated) -> { let children = vector.set(left_len, left_children, updated) - let last_size = vector.get(left_len, sizes) + size(updated) + let last_size = case left_len { + 1 -> size(updated) + _ -> vector.get(left_len - 1, sizes) + size(updated) + } let sizes = vector.set(left_len, sizes, last_size) Concatenated(Unbalanced(children:, sizes:)) } @@ -954,6 +959,17 @@ pub type SplitResult(item) { } pub fn split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { + case do_split(shift, node, index) { + Split(prefix:, prefix_shift:, suffix:, suffix_shift:) -> { + let #(prefix, prefix_shift) = compact_root(prefix, prefix_shift) + let #(suffix, suffix_shift) = compact_root(suffix, suffix_shift) + Split(prefix:, prefix_shift:, suffix:, suffix_shift:) + } + EmptyPrefix -> EmptyPrefix + } +} + +fn do_split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { let child_shift = shift - branch_bits case node { @@ -963,49 +979,30 @@ pub fn split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { let child = vector.get(node_index + 1, children) - case split(child_shift, child, index) { + case do_split(child_shift, child, index) { EmptyPrefix if node_index == 0 -> EmptyPrefix EmptyPrefix -> { let #(before_children, after_children) = vector.split(node_index + 1, children) let prefix = balanced(shift, before_children) + let suffix = balanced(shift, after_children) - let after_children_len = vector.length(after_children) - let suffix = case after_children_len { - 1 -> vector.get(1, after_children) - _ -> balanced(shift, after_children) - } - let suffix_shift = case after_children_len { - 1 -> child_shift - _ -> shift - } - - Split(prefix:, prefix_shift: shift, suffix:, suffix_shift:) + Split(prefix:, prefix_shift: shift, suffix:, suffix_shift: shift) } - Split(prefix:, prefix_shift:, suffix:, suffix_shift:) -> { + Split(prefix:, prefix_shift: _, suffix:, suffix_shift: _) -> { let #(before_children, after_children) = vector.split(node_index + 1, children) let before_children_len = vector.length(before_children) let prefix = case before_children_len { - 0 -> prefix + 0 -> balanced(shift, vector.singleton(prefix)) _ -> balanced(shift, vector.append(before_children, prefix)) } - let prefix_shift = case before_children_len { - 0 -> prefix_shift - _ -> shift - } + let prefix_shift = shift - let after_children_len = vector.length(after_children) - let suffix = case after_children_len { - 1 -> suffix - _ -> branch(shift, vector.set(1, after_children, suffix)) - } - let suffix_shift = case after_children_len { - 1 -> suffix_shift - _ -> shift - } + let suffix = branch(shift, vector.set(1, after_children, suffix)) + let suffix_shift = shift Split(prefix:, prefix_shift:, suffix:, suffix_shift:) } @@ -1022,7 +1019,7 @@ pub fn split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { let child = vector.get(node_index + 1, children) - case split(child_shift, child, index) { + case do_split(child_shift, child, index) { EmptyPrefix if node_index == 0 -> EmptyPrefix EmptyPrefix -> { let #(before_children, after_children) = @@ -1035,27 +1032,19 @@ pub fn split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { let prefix = unbalanced(shift, children: before_children, sizes: before_sizes) + let suffix = + unbalanced(shift, children: after_children, sizes: after_sizes) - let after_children_len = vector.length(after_children) - let suffix = case after_children_len { - 1 -> vector.get(1, after_children) - _ -> unbalanced(shift, children: after_children, sizes: after_sizes) - } - let suffix_shift = case after_children_len { - 1 -> child_shift - _ -> shift - } - - Split(prefix:, prefix_shift: shift, suffix:, suffix_shift:) + Split(prefix:, prefix_shift: shift, suffix:, suffix_shift: shift) } - Split(prefix:, prefix_shift:, suffix:, suffix_shift:) -> { + Split(prefix:, prefix_shift: _, suffix:, suffix_shift: _) -> { let #(before_children, after_children) = vector.split(node_index + 1, children) - let #(before_sizes, after_sizes) = vector.split(node_index + 1, sizes) + let #(before_sizes, _) = vector.split(node_index + 1, sizes) let before_children_len = vector.length(before_children) let prefix = case before_children_len { - 0 -> prefix + 0 -> balanced(shift, vector.singleton(prefix)) _ -> { let children = vector.append(before_children, prefix) let before_size = case node_index { @@ -1067,25 +1056,10 @@ pub fn split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { unbalanced(shift, children:, sizes:) } } - let prefix_shift = case before_children_len { - 0 -> prefix_shift - _ -> shift - } + let prefix_shift = shift - let after_children_len = vector.length(after_children) - let suffix = case after_children_len { - 1 -> suffix - _ -> { - let children = vector.set(1, after_children, suffix) - let after_delta = size(suffix) - vector.get(1, after_sizes) - let sizes = vector.map_add(after_sizes, after_delta) - unbalanced(shift, children:, sizes:) - } - } - let suffix_shift = case after_children_len { - 1 -> suffix_shift - _ -> shift - } + let suffix = branch(shift, vector.set(1, after_children, suffix)) + let suffix_shift = shift Split(prefix:, prefix_shift:, suffix:, suffix_shift:) } @@ -1105,6 +1079,23 @@ pub fn split(shift: Int, node: Node(item), index: Int) -> SplitResult(item) { } } +fn compact_root(node, shift) { + case node, shift > 0 { + Balanced(children:, ..), True -> + compact_children_root(node, children, shift) + Unbalanced(children:, ..), True -> + compact_children_root(node, children, shift) + _, _ -> #(node, shift) + } +} + +fn compact_children_root(node, children, shift) { + case vector.length(children) { + 1 -> compact_root(vector.get(1, children), shift - branch_bits) + _ -> #(node, shift) + } +} + pub fn map(node: Node(a), fun: fn(a) -> b) -> Node(b) { case node { Balanced(children:, size:) -> diff --git a/test/iv_test.gleam b/test/iv_test.gleam index 8fa2ff6..db047a2 100644 --- a/test/iv_test.gleam +++ b/test/iv_test.gleam @@ -69,6 +69,48 @@ pub fn split_concat2_test() { |> equal_arrays(array) } +pub fn insert_list_concat_rebalance_regression_test() { + let prefix = iv.concat(iv.repeat(0, times: 80), iv.repeat(0, times: 80)) + + prefix + |> iv.insert_list_clamped(at: 45, these: list.repeat(0, times: 3)) + |> iv.insert_clamped(at: 162, this: 0) + |> iv.insert_list_clamped(at: 0, these: list.repeat(0, times: 5)) + |> iv.insert_clamped(at: 167, this: 0) + |> iv.insert_list_clamped(at: 14, these: list.repeat(0, times: 55)) + |> iv.insert_clamped(at: 58, this: 0) + |> iv.insert_clamped(at: 80, this: 0) + |> iv.insert_clamped(at: 224, this: 0) + |> iv.insert_clamped(at: 223, this: 0) + |> iv.size + |> should.equal(229) +} + +pub fn repeated_insert_clamped_order_regression_test() { + iv.from_list([0, 11, 3, 13, 6, 9, 10, 12, 4, 5, 15, 1, 7, 8, 2, 14]) + |> iv.insert_clamped(at: 11, this: 16) + |> iv.insert_clamped(at: 17, this: 17) + |> iv.insert_clamped(at: 4, this: 18) + |> iv.insert_clamped(at: 15, this: 19) + |> iv.insert_clamped(at: 16, this: 20) + |> iv.insert_clamped(at: 6, this: 21) + |> iv.insert_clamped(at: 3, this: 22) + |> iv.insert_clamped(at: 2, this: 23) + |> iv.insert_clamped(at: 19, this: 24) + |> iv.insert_clamped(at: 20, this: 25) + |> iv.insert_clamped(at: 17, this: 26) + |> iv.insert_clamped(at: 22, this: 27) + |> iv.insert_clamped(at: 19, this: 28) + |> iv.insert_clamped(at: 18, this: 29) + |> iv.insert_clamped(at: 29, this: 30) + |> iv.insert_clamped(at: 31, this: 31) + |> iv.insert_clamped(at: 32, this: 32) + |> test_utils.equal_elements([ + 0, 11, 23, 3, 22, 13, 18, 6, 21, 9, 10, 12, 4, 5, 15, 16, 1, 26, 29, 7, 28, + 19, 24, 25, 27, 20, 8, 2, 14, 30, 17, 31, 32, + ]) +} + pub fn split_test() { let sizes = [10, 100, 1000, 10_000] use size <- list.each(sizes)