Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Cows with arbitrary lifetimes #89

Closed
wants to merge 1 commit into from
Closed

Support Cows with arbitrary lifetimes #89

wants to merge 1 commit into from

Conversation

hcsch
Copy link

@hcsch hcsch commented Aug 25, 2021

Fixes #86.

Since the lifetime parameter of Cow is only for the borrowed variant, it is irrelevant for the impl. This also allows for Cow<'static, str> as can be seen when testing with the following code:

#[derive(Arbitrary)]
pub struct StaticCow {
    cow: Cow<'static, str>,
}

#[test]
fn static_cow() {
    // Last byte is used for length
    let raw: Vec<u8> = vec![97, 98, 99, 100, 3];
    let static_cow: StaticCow = arbitrary_from(&raw);
    assert_eq!(
        Cow::<'static, str>::Owned("abc".to_string()),
        static_cow.cow
    );

    let (lower, upper) = <StaticCow as Arbitrary>::size_hint(0);
    assert_eq!(lower, 8);
    assert_eq!(upper, None);
}

If desirable I can also add this test case to tests/derive.rs (which is also where arbitrary_from is taken from).

@hcsch hcsch mentioned this pull request Aug 25, 2021
Merged
@nagisa
Copy link
Member

nagisa commented Aug 25, 2021

This seems okay at a first glance but I wonder if we might want to actually implement Cow::Borrowed stuff later down the line. It seems like an average fuzzer may be have an easier time seeing through the Cow if it didn't also involve an allocation for a typical str.

@hcsch
Copy link
Author

hcsch commented Aug 25, 2021

That is a good point you have there, I didn't really think about that. Thanks for bringing it up!

@hcsch
Copy link
Author

hcsch commented Aug 25, 2021 •
edited

I suppose an implementation for Cow<'static, T> would be in conflict with such a plan, at least as long as one can't specialize implementations based on lifetimes (edit: e.g. one impl for 'static, one for arbitrary 'a). Should I close this then @nagisa?

@hcsch hcsch closed this Aug 26, 2021
@hcsch hcsch deleted the arbitrary-lifetime-cow branch August 26, 2021 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deriving Arbitrary on a field containing a Cow<'static, str> fails.
2 participants
@hcsch @nagisa