Summary
Vector stores its length in an Int, and concat adds the two lengths with
no overflow check. Because concat shares structure, doubling a vector is
O(log n) — so a vector with more than 2^31 elements is cheap to build, and
length() then goes negative and is_empty() starts returning true for a
non-empty vector.
Reproducer
test {
let mut v = @vector.singleton(0)
for _ in 0..<30 {
v = v.concat(v)
}
println(v.length()) // 1073741824 (2^30, fine)
let a = v.concat(v)
println(a.length()) // -2147483648
let b = a.concat(a)
println("\{b.length()} \{b.is_empty()}") // 0 true
}
Each doubling is a single concat, so this runs in milliseconds and allocates
almost nothing — no 2 GB array is ever materialised.
Notes
Every Int-indexed collection shares this ceiling, so this may well be
"working as intended". It is worth a separate look here because structural
sharing removes the usual practical barrier: for Array you would have to
actually allocate the memory first, while for Vector you cannot. The internal
Tree::size arithmetic overflows alongside it.
Options, roughly in increasing cost: document the ceiling; abort on overflow
in concat/push; or widen the length. Filing so the trade-off is a decision
rather than an accident.
Found while reviewing immut/vector (see also #4083, #4084). Not addressed by
the PR that fixes those two, since the right answer here is a design call.
Summary
Vectorstores its length in anInt, andconcatadds the two lengths withno overflow check. Because
concatshares structure, doubling a vector isO(log n) — so a vector with more than 2^31 elements is cheap to build, and
length()then goes negative andis_empty()starts returningtruefor anon-empty vector.
Reproducer
Each doubling is a single
concat, so this runs in milliseconds and allocatesalmost nothing — no 2 GB array is ever materialised.
Notes
Every
Int-indexed collection shares this ceiling, so this may well be"working as intended". It is worth a separate look here because structural
sharing removes the usual practical barrier: for
Arrayyou would have toactually allocate the memory first, while for
Vectoryou cannot. The internalTree::sizearithmetic overflows alongside it.Options, roughly in increasing cost: document the ceiling;
aborton overflowin
concat/push; or widen the length. Filing so the trade-off is a decisionrather than an accident.
Found while reviewing
immut/vector(see also #4083, #4084). Not addressed bythe PR that fixes those two, since the right answer here is a design call.