Stream: git-wasmtime

Topic: wasmtime / PR #8381 Remove the `MmapVec::split_off` method

Wasmtime GitHub notifications bot (Apr 16 2024 at 14:33):

alexcrichton requested fitzgen for a review on PR #8381.

Wasmtime GitHub notifications bot (Apr 16 2024 at 14:33):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8381.

Wasmtime GitHub notifications bot (Apr 16 2024 at 14:33):

alexcrichton opened PR #8381 from alexcrichton:remove-mmap-vec-split-off to bytecodealliance:main:

This method isn't actually correct if split_off is used on the returned values again as it's missing some extra indexing processing. That being said nothing in Wasmtime currently uses it, so remove it entirely instead of keeping it around.

found by @FrankReh, thanks!

Wasmtime GitHub notifications bot (Apr 16 2024 at 14:34):

alexcrichton commented on PR #8381:

Notably @FrankReh recommended replacing with this:

pub fn split_off(&mut self, at: usize) -> MmapVec {
    assert!(at <= self.range.len());

    // Adjust the start of the range for the new part
    let new_start = self.range.start + at;

    // Create a new `MmapVec` which refers to the same underlying mmap, but
    // has a disjoint range from ours. Our own range is adjusted to be
    // disjoint just after `ret` is created.
    let ret = MmapVec {
        mmap: self.mmap.clone(),
        range: new_start..self.range.end,
    };
    self.range.end = new_start;
    return ret;
}

which I believe is correct, but also given that we don't need this it seems best to remove for now.

Wasmtime GitHub notifications bot (Apr 16 2024 at 15:10):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 16 2024 at 18:40):

alexcrichton merged PR #8381.

Last updated: Dec 23 2024 at 13:07 UTC