Implement Arbitrary for &[u8].

[frewsxcv]

#62 (comment)

[nagisa]

Consuming the entire unstructured buffer every time would mean that e.g.

#[derive(Arbitrary)]
struct Foo<'a> {
    bytes: &'a [u8],
    other_bytes: &'a [u9],
}

would always generate an empty slice for the 2nd field. You may want to do the arbitrary_len thing still here.

[fitzgen]

Good eye @nagisa. Yes, we should still do the arbitrary_len thing, but also implement arbitrary_take_rest with this implementation that takes all the bytes.

[frewsxcv]

Consuming the entire unstructured buffer every time would mean that e.g.

#[derive(Arbitrary)]
struct Foo<'a> {
    bytes: &'a [u8],
    other_bytes: &'a [u9],
}

would always generate an empty slice for the 2nd field. You may want to do the arbitrary_len thing still here.

great catch! fixed in f44a7e0

Good eye @nagisa. Yes, we should still do the arbitrary_len thing, but also implement arbitrary_take_rest with this implementation that takes all the bytes.

the default implementation for arbitrary_take_rest defers to arbitrary:

arbitrary/src/lib.rs

Lines 173 to 180 in f951b5a

	/// Generate an arbitrary value of `Self` from the entirety of the given unstructured data.
	///
	/// This is similar to Arbitrary::arbitrary, however it assumes that it is the
	/// last consumer of the given data, and is thus able to consume it all if it needs.
	/// See also the documentation for [`Unstructured`][crate::Unstructured].
	fn arbitrary_take_rest(mut u: Unstructured<'a>) -> Result<Self> {
	Self::arbitrary(&mut u)
	}

so seems like it'll already use this code. is that what you were thinking?

[fitzgen]

Not quite. It should be this:

impl<'a> Arbitrary<'a> for &'a [u8] {
    // ...
    fn arbitrary_take_rest(mut u: Unstructured<'a>) -> Result<Self> {
        Ok(u.take_rest())
    }
}

This will always consume the rest of the Unstructured's input (which is what we want in the _take_rest method) without getting any kind of length prefix (we only want the length prefix in regular arbitrary, not in arbitrary_take_rest).

Does that clear things up? Sorry this required so many back and forths to spell out.

[frewsxcv]

@nagisa @fitzgen Thanks for your patience, this is ready for another review. Just added a couple more commits

[fitzgen]

LGTM modulo the arbitrary_take_rest stuff upthread and a question about the test below.

[fitzgen]

Does this test still pass with the length prefix stuff? I expect it to pass when using arbitrary_take_rest but not with regular arbitrary, where I instead I would expect that we read a length (which is read from the end of the data) and then get some prefix of [1, 2, 3] depending on how arbitrary_len maps 4 into the 0..3 range.

[frewsxcv]

Check the latest commit– does that match what's in your head?

[fitzgen]

Perfect! Thanks @frewsxcv :)