Skip to content

Commit

Permalink
Merge pull request #384 from lorentey/rope-cow-violation
Browse files Browse the repository at this point in the history
[Rope] Fix copy-on-write violation in Rope.join
  • Loading branch information
lorentey authored Jun 3, 2024
2 parents e166904 + a0b839d commit 8253d6d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 4 deletions.
11 changes: 8 additions & 3 deletions Sources/RopeModule/Rope/Operations/Rope+Join.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ extension Rope {
var left = left.root
var right = right.root

left.ensureUnique()
right.ensureUnique()

if left.height >= right.height {
let r = left._graftBack(&right)
guard let remainder = r.remainder else { return Self(root: left) }
Expand All @@ -64,6 +61,10 @@ extension Rope._Node {
_ scion: inout Self
) -> (remainder: Self?, delta: Summary) {
assert(self.height >= scion.height)

self.ensureUnique()
scion.ensureUnique()

guard self.height > scion.height else {
assert(self.height == scion.height)
let d = scion.summary
Expand Down Expand Up @@ -99,6 +100,10 @@ extension Rope._Node {
_ scion: inout Self
) -> (remainder: Self?, delta: Summary) {
assert(self.height >= scion.height)

self.ensureUnique()
scion.ensureUnique()

guard self.height > scion.height else {
assert(self.height == scion.height)
let origSum = self.summary
Expand Down
23 changes: 22 additions & 1 deletion Tests/RopeModuleTests/TestBigString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,28 @@ class TestBigString: CollectionTestCase {
}
return pieces
}


func test_append_copy_on_write() {
let flat = String(repeating: sampleString, count: 10)
withEvery("stride", in: [32, 64, 128, 250, 1_000, 10_000, 20_000]) { stride in
let pieces = self.pieces(of: flat, by: stride).map {
BigString($0.str)
}

var big: BigString = ""
withEvery("i", in: 0 ..< pieces.count) { i in
let copy = big
let expected = String(copy)

big.append(contentsOf: pieces[i])

let actual = String(copy)
expectTrue(actual == expected)
copy._invariantCheck()
}
}
}

func test_insert_string() {
let flat = sampleString
let ref = BigString(flat)
Expand Down
32 changes: 32 additions & 0 deletions Tests/RopeModuleTests/TestRope.swift
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,38 @@ class TestRope: CollectionTestCase {
expectEqual(ranges, [0 ..< c])
}

func test_join_copy_on_write() {
let c = 10_000
var trees = (0 ..< c).map {
let chunk = Chunk(length: ($0 % 4) + 1, value: $0)
return Rope(CollectionOfOne(chunk))
}
var ranges = (0 ..< c).map { $0 ..< $0 + 1 }

var rng = RepeatableRandomNumberGenerator(seed: 0)
while trees.count >= 2 {
let i = (0 ..< trees.count - 1).randomElement(using: &rng)!
let expectedRange = ranges[i].lowerBound ..< ranges[i + 1].upperBound

let a = trees[i]
let b = trees.remove(at: i + 1)
trees[i] = Rope()

let joined = Rope.join(a, b)

expectEqualElements(a.map { $0.value }, Array(ranges[i]), "Copy-on-write violation")
expectEqualElements(b.map { $0.value }, Array(ranges[i + 1]), "Copy-on-write violation")

a._invariantCheck()
b._invariantCheck()
joined._invariantCheck()

trees[i] = joined
ranges.replaceSubrange(i ... i + 1, with: CollectionOfOne(expectedRange))
}
expectEqual(ranges, [0 ..< c])
}

func chunkify(_ values: [Int]) -> [Chunk] {
var result: [Chunk] = []
var last = Int.min
Expand Down

0 comments on commit 8253d6d

Please sign in to comment.