Make <&str as Arbitrary>::arbitrary_take_rest less likely to fail #151
Conversation
Before the changes, if any character in the remaining data were to be invalid utf8, it would fail to generate a string. After the change, it takes all it can up to the first invalid utf8 byte.
|
Done, and checked that the test fails before the change and passes after it :) |
| let i = e.valid_up_to(); | ||
| let valid = u.bytes(i).unwrap(); | ||
| let s = unsafe { | ||
| debug_assert!(str::from_utf8(valid).is_ok()); |
There was a problem hiding this comment.
thought: should we be including this? fuzzing is usually run with debug assertions on so the debug_assert doesn't get us much. I am comfortable with more thoroughly documenting the safety situation instead
There was a problem hiding this comment.
We have a couple other debug asserts that also maybe should only be enabled when running the arbitrary crate tests. We can figure out what to do with all of them in a follow up, no need to hold this PR up.
There was a problem hiding this comment.
This debug_assert comes from the previous implementation of arbitrary; if you think it’d be better to replace it with a comment that valid_up_to can only generate valid strings, or with another macro that’d assert only on eg. an internal-tests arbitrary feature, I’d be happy to make a follow-up PR :)
Before the changes, if any character in the remaining data were to be invalid utf8, it would fail to generate a string.
After the change, it takes all it can up to the first invalid utf8 byte.