Is this splice test correct?

help
1

I just want to check my understanding here.

I am now starting to test my Vec implementation using the standard tests, one test is

#[test]
fn test_splice_forget() {
    let mut v = vec![1, 2, 3, 4, 5];
    let a = [10, 11, 12];
    std::mem::forget(v.splice(2..4, a));
    assert_eq!(v, &[1, 2]);
}

But the documentation says:

It is unspecified how many elements are removed from the vector if the Splice value is leaked.

So how is that test valid? Or is there a sort of hidden specification on how it “really works”, or perhaps the test is making sure the way it works doesn’t change?

2

Not every unit test of the standard library is written in the fashion of a spec-compliance test. They may be asserting the exact behavior of the current implementation, because that is, in most cases, easier to maintain than one which more precisely checks the documented guarantees and no more.

5 Likes
3

Thanks, I just wanted to be sure I wasn’t mis-understanding the situation, and especially that I hadn’t completely gone crazy and misunderstood what the documentation says! Perhaps a comment on the test to mention that is is not specification-based in this instance would be helpful. Or maybe it is obvious!

[ Incidentally, leaking the Splice seems to me to be a very strange and unlikely thing for anyone to do ]

1 Like
4

Something it's important to remember is that leaking is a broad concept. Sometimes it's relevant only because of the safety implications re. lifetimes, but otherwise the question "what happens when X leaks" is equivalent to "what happens when X is accidentally held onto longer than expected".

5

I think in this case, if it leaks in the sense that it lives forever, then because the Vec is mutably borrowed by the Splice, the state of the Vec ( what elements have been removed from it ), is moot, because it cannot be observed… does that make sense?

6

Sure, but the goal isn't to be sound in the face of likely and non-strange code, the goal is to sound in the face of all safe code. And you can leak (not run the destructor of) Splice in safe code, so that possibility needs to be accounted for.

Read this article for more context on destructor leaking safety. In particular, splice is in a similar boat as the drain_range example (which became drain).

1 Like
7

Yeah, in this case it's not particularly relevant; I should've made that clear. My point was more about the general case.

8

There is a hint in there:

// Tell the Vec that it doesn't have any elements
// stored after the minimum bound.
unsafe { self.set_len(a); }

I have chosen to simply set it to zero, so I didn’t pass the splice_forget test. I guess I may revisit that.

It seemed slightly more logical to me to temporarily “take ownership” of the entire Vec, rather than just part of it.