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

CHANGELOG.md

@@ -28,6 +28,26 @@ Released YYYY-MM-DD.
 
 --------------------------------------------------------------------------------
 
+## 1.0.0-rc1
+
+Unreleased.
+
+### Changed
+
+* The `Arbitrary` trait is now implemented for `&str`. [#63](https://github.com/rust-fuzz/arbitrary/pull/63)
+
+### Changed
+
+* The `Arbitrary` trait now has a lifetime parameter, allowing `Arbitrary` implementations that borrow from the raw input (e.g. the new `&str` implementaton). The `derive(Arbitrary)` macro also supports deriving `Arbitrary` on types with lifetimes now. [#63](https://github.com/rust-fuzz/arbitrary/pull/63)
+
+### Removed
+
+* The `shrink` method on the `Arbitrary` trait has been removed.
+
+  We have found that, in practice, using [internal reduction](https://drmaciver.github.io/papers/reduction-via-generation-preview.pdf) via approaches like `cargo fuzz tmin`, where the raw input bytes are reduced rather than the `T: Arbitrary` type constructed from those raw bytes, has the best efficiency-to-maintenance ratio. To the best of our knowledge, no one is relying on or using the `Arbitrary::shrink` method. If you *are* using and relying on the `Arbitrary::shrink` method, please reach out by [dropping a comment here](https://github.com/rust-fuzz/arbitrary/issues/62) and explaining how you're using it and what your use case is. We'll figure out what the best solution is, including potentially adding shrinking functionality back to the `arbitrary` crate.
+
+--------------------------------------------------------------------------------
+
 ## 0.4.7
 
 Released 2020-10-14.

README.md

@@ -77,8 +77,8 @@ pub struct Rgb {
     pub b: u8,
 }
 
-impl Arbitrary for Rgb {
-    fn arbitrary(u: &mut Unstructured<'_>) -> Result<Self> {
+impl<'a> Arbitrary<'a> for Rgb {
+    fn arbitrary(u: &mut Unstructured<'a>) -> Result<Self> {
         let r = u8::arbitrary(u)?;
         let g = u8::arbitrary(u)?;
         let b = u8::arbitrary(u)?;
@@ -87,33 +87,6 @@ impl Arbitrary for Rgb {
 }
 ```
 
-### Shrinking
-
-To assist with test case reduction, where you want to find the smallest and most
-easily understandable test case that still demonstrates a bug you've discovered,
-the `Arbitrary` trait has a `shrink` method. The `shrink` method returns an
-iterator of "smaller" instances of `self`. The provided, default implementation
-returns an empty iterator.
-
-We can override the default for our `Rgb` struct above by shrinking each of its
-components and then gluing them back together again:
-
-```rust
-impl Arbitrary for Rgb {
-    // ...
-
-    fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
-        let rs = self.r.shrink();
-        let gs = self.g.shrink();
-        let bs = self.b.shrink();
-        Box::new(rs.zip(gs).zip(bs).map(|((r, g), b)| Rgb { r, g, b }))
-    }
-}
-```
-
-Note that deriving `Arbitrary` will automatically derive a custom `shrink`
-implementation for you.
-
 ## License
 
 Licensed under dual MIT or Apache-2.0 at your choice.

derive/src/lib.rs

@@ -1,51 +1,83 @@
 extern crate proc_macro;
 
-use proc_macro2::{Literal, TokenStream};
+use proc_macro2::{Span, TokenStream};
 use quote::quote;
 use syn::*;
 
+static ARBITRARY_LIFETIME_NAME: &str = "'arbitrary";
+
 #[proc_macro_derive(Arbitrary)]
 pub fn derive_arbitrary(tokens: proc_macro::TokenStream) -> proc_macro::TokenStream {
     let input = syn::parse_macro_input!(tokens as syn::DeriveInput);
+    let (lifetime_without_bounds, lifetime_with_bounds) =
+        build_arbitrary_lifetime(input.generics.clone());
 
-    let arbitrary_method = gen_arbitrary_method(&input);
+    let arbitrary_method = gen_arbitrary_method(&input, lifetime_without_bounds.clone());
     let size_hint_method = gen_size_hint_method(&input);
-    let shrink_method = gen_shrink_method(&input);
     let name = input.ident;
     // Add a bound `T: Arbitrary` to every type parameter T.
-    let generics = add_trait_bounds(input.generics);
-    let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
+    let generics = add_trait_bounds(input.generics, lifetime_without_bounds.clone());
+
+    // Build ImplGeneric with a lifetime (https://github.com/dtolnay/syn/issues/90)
+    let mut generics_with_lifetime = generics.clone();
+    generics_with_lifetime
+        .params
+        .push(GenericParam::Lifetime(lifetime_with_bounds));
+    let (impl_generics, _, _) = generics_with_lifetime.split_for_impl();
+
+    // Build TypeGenerics and WhereClause without a lifetime
+    let (_, ty_generics, where_clause) = generics.split_for_impl();
+
     (quote! {
-        impl #impl_generics arbitrary::Arbitrary for #name #ty_generics #where_clause {
+        impl #impl_generics arbitrary::Arbitrary<#lifetime_without_bounds> for #name #ty_generics #where_clause {
             #arbitrary_method
             #size_hint_method
-            #shrink_method
         }
     })
     .into()
 }
 
+// Returns: (lifetime without bounds, lifetime with bounds)
+// Example: ("'arbitrary", "'arbitrary: 'a + 'b")
+fn build_arbitrary_lifetime(generics: Generics) -> (LifetimeDef, LifetimeDef) {
+    let lifetime_without_bounds =
+        LifetimeDef::new(Lifetime::new(ARBITRARY_LIFETIME_NAME, Span::call_site()));
+    let mut lifetime_with_bounds = lifetime_without_bounds.clone();
+
+    for param in generics.params.iter() {
+        if let GenericParam::Lifetime(lifetime_def) = param {
+            lifetime_with_bounds
+                .bounds
+                .push(lifetime_def.lifetime.clone());
+        }
+    }
+
+    (lifetime_without_bounds, lifetime_with_bounds)
+}
+
 // Add a bound `T: Arbitrary` to every type parameter T.
-fn add_trait_bounds(mut generics: Generics) -> Generics {
+fn add_trait_bounds(mut generics: Generics, lifetime: LifetimeDef) -> Generics {
     for param in generics.params.iter_mut() {
         if let GenericParam::Type(type_param) = param {
-            type_param.bounds.push(parse_quote!(arbitrary::Arbitrary));
+            type_param
+                .bounds
+                .push(parse_quote!(arbitrary::Arbitrary<#lifetime>));
         }
     }
     generics
 }
 
-fn gen_arbitrary_method(input: &DeriveInput) -> TokenStream {
+fn gen_arbitrary_method(input: &DeriveInput, lifetime: LifetimeDef) -> TokenStream {
     let ident = &input.ident;
     let arbitrary_structlike = |fields| {
         let arbitrary = construct(fields, |_, _| quote!(arbitrary::Arbitrary::arbitrary(u)?));
         let arbitrary_take_rest = construct_take_rest(fields);
         quote! {
-            fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result<Self> {
+            fn arbitrary(u: &mut arbitrary::Unstructured<#lifetime>) -> arbitrary::Result<Self> {
                 Ok(#ident #arbitrary)
             }
 
-            fn arbitrary_take_rest(mut u: arbitrary::Unstructured<'_>) -> arbitrary::Result<Self> {
+            fn arbitrary_take_rest(mut u: arbitrary::Unstructured<#lifetime>) -> arbitrary::Result<Self> {
                 Ok(#ident #arbitrary_take_rest)
             }
         }
@@ -70,7 +102,7 @@ fn gen_arbitrary_method(input: &DeriveInput) -> TokenStream {
             });
             let count = data.variants.len() as u64;
             quote! {
-                fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result<Self> {
+                fn arbitrary(u: &mut arbitrary::Unstructured<#lifetime>) -> arbitrary::Result<Self> {
                     // Use a multiply + shift to generate a ranged random number
                     // with slight bias. For details, see:
                     // https://lemire.me/blog/2016/06/30/fast-random-shuffling
@@ -80,7 +112,7 @@ fn gen_arbitrary_method(input: &DeriveInput) -> TokenStream {
                     })
                 }
 
-                fn arbitrary_take_rest(mut u: arbitrary::Unstructured<'_>) -> arbitrary::Result<Self> {
+                fn arbitrary_take_rest(mut u: arbitrary::Unstructured<#lifetime>) -> arbitrary::Result<Self> {
                     // Use a multiply + shift to generate a ranged random number
                     // with slight bias. For details, see:
                     // https://lemire.me/blog/2016/06/30/fast-random-shuffling
@@ -162,87 +194,3 @@ fn gen_size_hint_method(input: &DeriveInput) -> TokenStream {
         }
     }
 }
-
-fn gen_shrink_method(input: &DeriveInput) -> TokenStream {
-    let ident = &input.ident;
-    let shrink_structlike = |fields| {
-        let inner = shrink(&quote!(#ident), fields, |i, field| match &field.ident {
-            Some(i) => quote!(&self.#i),
-            None => {
-                let i = Literal::usize_unsuffixed(i);
-                quote!(&self.#i)
-            }
-        });
-        quote! {
-            fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
-                #inner
-            }
-        }
-    };
-
-    return match &input.data {
-        Data::Struct(data) => shrink_structlike(&data.fields),
-        Data::Union(data) => shrink_structlike(&Fields::Named(data.fields.clone())),
-        Data::Enum(data) => {
-            let variants = data.variants.iter().map(|variant| {
-                let mut binding_names = Vec::new();
-                let bindings = match &variant.fields {
-                    Fields::Named(_) => {
-                        let names = variant.fields.iter().map(|f| {
-                            let name = f.ident.as_ref().unwrap();
-                            binding_names.push(quote!(#name));
-                            name
-                        });
-                        quote!({#(#names),*})
-                    }
-                    Fields::Unnamed(_) => {
-                        let names = (0..variant.fields.len()).map(|i| {
-                            let name = quote::format_ident!("f{}", i);
-                            binding_names.push(quote!(#name));
-                            name
-                        });
-                        quote!((#(#names),*))
-                    }
-                    Fields::Unit => quote!(),
-                };
-                let variant_name = &variant.ident;
-                let shrink = shrink(&quote!(#ident::#variant_name), &variant.fields, |i, _| {
-                    binding_names[i].clone()
-                });
-                quote!(#ident::#variant_name #bindings => { #shrink })
-            });
-            quote! {
-                fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
-                    match self {
-                        #(#variants)*
-                    }
-                }
-            }
-        }
-    };
-
-    fn shrink(
-        prefix: &TokenStream,
-        fields: &Fields,
-        access_field: impl Fn(usize, &Field) -> TokenStream,
-    ) -> TokenStream {
-        if fields.len() == 0 {
-            return quote!(Box::new(None.into_iter()));
-        }
-        let iters = fields.iter().enumerate().map(|(i, f)| {
-            let name = quote::format_ident!("field{}", i);
-            let field = access_field(i, f);
-            quote! { let mut #name = arbitrary::Arbitrary::shrink(#field); }
-        });
-        let ctor = construct(fields, |i, _| {
-            let iter = quote::format_ident!("field{}", i);
-            quote!(#iter.next()?)
-        });
-        quote! {
-                #(#iters)*
-                Box::new(std::iter::from_fn(move || {
-                    Some(#prefix #ctor)
-                }))
-        }
-    }
-}