Add lifetime parameter to Arbitrary trait. Remove shrinking functionality. Implement Arbitrary for &str.

[frewsxcv]

Fixes #43

Note: This merges into a new branch staging-1.0 where we can put our changes prepping for a 1.0 release

To-do:

• Remove Shrinkable
• Fix Cow lifetime
• Remove shrinking from README
• Save shrinking code to a gist

[frewsxcv]

@Manishearth @fitzgen thoughts on this so far? should we create a separate crate for the shrinking behavior?

[Manishearth]

I think we should have a pre-1.0 release before going 1.0 fwiw. But we can name the branch whatever

[fitzgen]

I'm not sure the shrinking code is worth maintaining? AFAIK, no one is using it at all. It isn't used with cargo fuzz, so someone would have to be implementing their own fuzzing engine or driver to use it, which AFAIK, no one is doing. As the person who wrote the shrinking code initially, I'd like to hear someone else champion it and articulate why we should keep it around.

In any case, if we keep it, then we should name the trait Shrink instead of Shrinkable, the same way std::iter::Extend is not named Extendable.

Finally, we should also add a impl<'a> Arbitrary<'a> for &'a str implementation, now that it's possible.

[fitzgen]

Also: thanks for driving this work @frewsxcv! :)

[frewsxcv]

I'm not using the shrinking code, and not planning to use it. Just moved the code to a separate module in case someone did have a use for it! But if none of us do, then yeah let's just get rid of it

[frewsxcv]

to anyone in the future who wants the shrinking code: https://gist.github.com/frewsxcv/390976eb39c7e2a065c0b2d2731a3eb3

[frewsxcv]

currently trying to figure out what the right behavior should be for the new lifetime when deriving Arbitrary. i.e. should we always create a lifetime named 'a? will that cause issues?

[fitzgen]

Do you mean the actual name of the lifetime we emit in the generated code?

The only way I can imagine running into issues is if the type requires multiple type parameters, but in such a case I don't think our derive can support such types anyways, because we wouldn't know what to do with any extra lifetimes.

But I'm also not a master macrologist, so I'm not 100% sure here...

[frewsxcv]

This is ready for review! I will add a &str implementation in a follow-up PR

[frewsxcv]

Squashed

[frewsxcv]

In the latest force push, I renamed our derive codegen to generate a lifetime named 'arbitrary instead of 'a. To prevent collisions if the user has a lifetime named 'a.

[fitzgen]

Nice!

Can we also have a type with a lifetime parameter that we derive Arbitrary for in the tests/derive.rs file?

With that, I think we can release a 1.0-rc1.

[fitzgen]

In the latest force push, I renamed our derive codegen to generate a lifetime named 'arbitrary instead of 'a. To prevent collisions if the user has a lifetime named 'a.

Oh I just remembered the other thing we can do: try and grab their existing lifetime from their type and use that instead of coming up with our own and replacing theirs. Sort of something like get_existing_lifetime(input).unwrap_or(default_lifetime).

[frewsxcv]

In the latest force push, I renamed our derive codegen to generate a lifetime named 'arbitrary instead of 'a. To prevent collisions if the user has a lifetime named 'a.

Oh I just remembered the other thing we can do: try and grab their existing lifetime from their type and use that instead of coming up with our own and replacing theirs. Sort of something like get_existing_lifetime(input).unwrap_or(default_lifetime).

I think you're right, we do want to be smarter about lifetimes than just using a new one. In particular, I created this struct for a test:

#[derive(Arbitrary, Debug)]
struct Lifetime<'a, 'b> {
    alpha: &'a str,
    beta: &'b str,
}

This results in an error as currently implemented:

The fix here is to replace:

impl<'a, 'b, 'arbitrary> arbitrary::Arbitrary<'arbitrary> for Lifetime<'a, 'b> 

With:

impl<'a, 'b, 'arbitrary: 'a + 'b> arbitrary::Arbitrary<'arbitrary> for Lifetime<'a, 'b> 

So that our lifetime is guaranteed to last as long as the existing lifetimes

[frewsxcv]

Also good idea to add a test :)

[fitzgen]

LGTM!

A couple inline suggestions, just little things.

With this merged, I think we can release 1.0.0-rc1, unless I'm overlooking something. @Manishearth @nagisa anything you can think of holding us back from a 1.0.0-rc1 release?

[Manishearth]

lgtm modulo nick's comments

[frewsxcv]

1.0.0-rc1 has been published!