Skip to content

Fix elements exceeding node size for unrolled lists - #19

Merged
mattwparas merged 3 commits into
mattwparas:masterfrom
dmun:master
Aug 19, 2026
Merged

mattwparas merged 3 commits into
mattwparas:masterfrom
dmun:master

Conversation

@dmun

@dmun dmun commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

On coalesce, the left and right nodes' locations and pointers are swapped, but not size, meaning that a node could have 6 elements but have size 4.

If you append this resulting node to an empty list, they don't coalesce and an empty node would remain as the head, while still linking to the other node, causing what you see below:

Before

(define (foo x)
  (append '() (reverse (range 0 x))))

;; 5 to 8 fail
(foo 4) ; => '(3 2 1 0)
(foo 5) ; => '()
(foo 6) ; => '()
(foo 7) ; => '()
(foo 8) ; => '()
(foo 9) ; => '(8 7 6 5 4 3 2 1 0)

;; contradicting length and empty?
(length (foo 6)) ; => 6
(empty? (foo 6)) ; => #true
type SmallRcList<T> = UnrolledList<T, RcPointer, 4, 2>;
let left: SmallRcList<i32> = SmallRcList::new();
let right: SmallRcList<_> = (0..6).collect::<SmallRcList<_>>().reverse();
let left = left.append(right);

left.node_iter().for_each(|node| {
    println!("{}/{}: {:?}", node.index(), node.size(), node.elements());
});

// Output:
// 0/4: []
// 6/4: [0, 1, 2, 3, 4, 5]

After

After adding the size swap + test

(foo 4) ; => '(3 2 1 0)
(foo 5) ; => '(4 3 2 1 0)
(foo 6) ; => '(5 4 3 2 1 0)
(foo 7) ; => '(6 5 4 3 2 1 0)
(foo 8) ; => '(7 6 5 4 3 2 1 0)
(foo 9) ; => '(8 7 6 5 4 3 2 1 0)

(length (foo 6)) ; => 6
(empty? (foo 6)) ; => #false
// same rust code

// Output:
// 6/8: [0, 1, 2, 3, 4, 5]

Notes

Also, seems like asserts_invariants() didn't call assert!, when I added it, more errors appeared. All seem fixed after my change.

failures:
    unrolled::iterator_tests::empty_node_appending_coalescing_works
    unrolled::proptests::vlist::append_non_mut_resulting_length_equivalent
    unrolled::proptests::vlist::append_resulting_length_equivalent
    unrolled::proptests::vlist::append_zero_then_popfront
    unrolled::proptests::vlist::operations_in_order_match
    unrolled::proptests::vlist::test_case_with_pop_front
    unrolled::proptests::vlist::test_case_with_reverse
    unrolled::proptests::vlist_growth_rate_4::append_non_mut_resulting_length_equivalent
    unrolled::proptests::vlist_growth_rate_4::append_resulting_length_equivalent
    unrolled::proptests::vlist_growth_rate_4::cdr_to_append
    unrolled::proptests::vlist_growth_rate_4::larger_test_case
    unrolled::proptests::vlist_growth_rate_4::operations_in_order_match
    unrolled::proptests::vlist_growth_rate_4::test_case_with_pop_front
    unrolled::proptests::vlist_growth_rate_4::test_case_with_reverse

test result: FAILED. 210 passed; 14 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.25s
test result: ok. 224 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.51s

@mattwparas mattwparas left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you! I'll publish a new release tonight

@mattwparas
mattwparas merged commit 9c18e86 into mattwparas:master Aug 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants