Repository: rust-bakery/nom (language: rust)

Issue:
`fold_many1` (and some others) now requires `Clone` attribute
- Rust version : 1.36
- nom version : 5.0

`fold_many1` now requires init/return value to have `Clone`, which is annoying when it can be a complex type (and you don't want to clone it).

It was not the case with nom 4.

## Test case

```rust
struct TypeWithoutClone {
    pub a: u32,
}

named!(multi<&[u8], Vec<TypeWithoutClone>>,
    fold_many1!( tag!( "abcd" ), Vec::new(), |mut acc: Vec<_>, item| {
        acc.push( TypeWithoutClone{ a: 0 } );
        acc
    })
);
```

Nom 4 build: no error

Nom 5 build:
```
error[E0277]: the trait bound `TypeWithoutClone: std::clone::Clone` is not satisfied
  --> src/lib.rs:14:1
   |
14 | / named!(multi<&[u8], Vec<TypeWithoutClone>>,
15 | |     fold_many1!( tag!( "abcd" ), Vec::new(), |mut acc: Vec<_>, item| {
16 | |         acc.push( TypeWithoutClone{ a: 0 } );
17 | |         acc
18 | |     })
19 | | );
   | |__^ the trait `std::clone::Clone` is not implemented for `TypeWithoutClone`
   |
   = note: required because of the requirements on the impl of `std::clone::Clone` for `std::vec::Vec<TypeWithoutClone>`
   = note: required by `nom::multi::fold_many1c`
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
```



Discussion / hints:
Agreed. Seems that `fold_many0()` and `fold_many1()` do the same, so I have switched to a manual combination of `many0()` and `map()` to improve performance on my end. Is there a particular reason why the `R: Clone` requirement was introduced in `nom` 5.0? CC @Geal 

**EDIT:** It seems it's because `fold_many0()` and `fold_many1()` return an `Fn` closure, which means any state carried over must be cloned or mutably borrowed.
Vendoring the `fold_many0()` combinator in a personal project, changing it to return an `impl FnOnce(I) -> IResult<I, O, E>`, and removing the `R: Clone` bound and `clone()` call seems to produce the desired result. The parser compiles and works as expected, but naturally, the disadvantage of using `FnOnce` is that the closure produced by the combinator can only be called once.
To add to @ebkalderon solution.
Its possible to make the Parser return `impl Fn(I) -> IResult<I, O, E>` again by using a function that supplies the initial accumulator `init: Fn() -> R` instead of passing it directly to the function. `fold_*` can then call this supplier from within the parser function to get a new accumulator making it `Fn` instead of `FnOnce`.
I have tested this briefly and found no issue with it.
[Example of modified `fold_many0` function.](https://gist.github.com/cenodis/f66e9f45eb304e3d7e31959b5df85101)
@cenodis that's a good approach, it eliminates the Clone trait bound with minimal modifications in code using fold_many0

Fix patch:
diff --git a/examples/string.rs b/examples/string.rs
index 73c5d3e59..2efd29e72 100644
--- a/examples/string.rs
+++ b/examples/string.rs
@@ -148,7 +148,7 @@ where
     // Our parser function– parses a single string fragment
     parse_fragment,
     // Our init value, an empty string
-    String::new(),
+    String::new,
     // Our folding function. For each fragment, append the fragment to the
     // string.
     |mut string, fragment| {
diff --git a/src/multi/mod.rs b/src/multi/mod.rs
index 99061d5c4..b00568c0e 100644
--- a/src/multi/mod.rs
+++ b/src/multi/mod.rs
@@ -596,7 +596,7 @@ where
 /// the results using a given function and initial value.
 /// # Arguments
 /// * `f` The parser to apply.
-/// * `init` The initial value.
+/// * `init` A function returning the initial value.
 /// * `g` The function that combines a result of `f` with
 ///       the current accumulator.
 /// ```rust
@@ -608,7 +608,7 @@ where
 /// fn parser(s: &str) -> IResult<&str, Vec<&str>> {
 ///   fold_many0(
 ///     tag("abc"),
-///     Vec::new(),
+///     Vec::new,
 ///     |mut acc: Vec<_>, item| {
 ///       acc.push(item);
 ///       acc
@@ -621,20 +621,20 @@ where
 /// assert_eq!(parser("123123"), Ok(("123123", vec![])));
 /// assert_eq!(parser(""), Ok(("", vec![])));
 /// ```
-pub fn fold_many0<I, O, E, F, G, R>(
+pub fn fold_many0<I, O, E, F, G, H, R>(
   mut f: F,
-  init: R,
+  mut init: H,
   mut g: G,
 ) -> impl FnMut(I) -> IResult<I, R, E>
 where
   I: Clone + PartialEq,
   F: Parser<I, O, E>,
   G: FnMut(R, O) -> R,
+  H: FnMut() -> R,
   E: ParseError<I>,
-  R: Clone,
 {
   move |i: I| {
-    let mut res = init.clone();
+    let mut res = init();
     let mut input = i;
 
     loop {
@@ -666,7 +666,7 @@ where
 /// once.
 /// # Arguments
 /// * `f` The parser to apply.
-/// * `init` The initial value.
+/// * `init` A function returning the initial value.
 /// * `g` The function that combines a result of `f` with
 ///       the current accumulator.
 /// ```rust
@@ -678,7 +678,7 @@ where
 /// fn parser(s: &str) -> IResult<&str, Vec<&str>> {
 ///   fold_many1(
 ///     tag("abc"),
-///     Vec::new(),
+///     Vec::new,
 ///     |mut acc: Vec<_>, item| {
 ///       acc.push(item);
 ///       acc
@@ -691,21 +691,21 @@ where
 /// assert_eq!(parser("123123"), Err(Err::Error(Error::new("123123", ErrorKind::Many1))));
 /// assert_eq!(parser(""), Err(Err::Error(Error::new("", ErrorKind::Many1))));
 /// ```
-pub fn fold_many1<I, O, E, F, G, R>(
+pub fn fold_many1<I, O, E, F, G, H, R>(
   mut f: F,
-  init: R,
+  mut init: H,
   mut g: G,
 ) -> impl FnMut(I) -> IResult<I, R, E>
 where
   I: Clone + PartialEq,
   F: Parser<I, O, E>,
   G: FnMut(R, O) -> R,
+  H: FnMut() -> R,
   E: ParseError<I>,
-  R: Clone,
 {
   move |i: I| {
     let _i = i.clone();
-    let init = init.clone();
+    let init = init();
     match f.parse(_i) {
       Err(Err::Error(_)) => Err(Err::Error(E::from_error_kind(i, ErrorKind::Many1))),
       Err(e) => Err(e),
@@ -745,7 +745,7 @@ where
 /// * `m` The minimum number of iterations.
 /// * `n` The maximum number of iterations.
 /// * `f` The parser to apply.
-/// * `init` The initial value.
+/// * `init` A function returning the initial value.
 /// * `g` The function that combines a result of `f` with
 ///       the current accumulator.
 /// ```rust
@@ -759,7 +759,7 @@ where
 ///     0,
 ///     2,
 ///     tag("abc"),
-///     Vec::new(),
+///     Vec::new,
 ///     |mut acc: Vec<_>, item| {
 ///       acc.push(item);
 ///       acc
@@ -773,22 +773,22 @@ where
 /// assert_eq!(parser(""), Ok(("", vec![])));
 /// assert_eq!(parser("abcabcabc"), Ok(("abc", vec!["abc", "abc"])));
 /// ```
-pub fn fold_many_m_n<I, O, E, F, G, R>(
+pub fn fold_many_m_n<I, O, E, F, G, H, R>(
   min: usize,
   max: usize,
   mut parse: F,
-  init: R,
+  mut init: H,
   mut fold: G,
 ) -> impl FnMut(I) -> IResult<I, R, E>
 where
   I: Clone + PartialEq,
   F: Parser<I, O, E>,
   G: FnMut(R, O) -> R,
+  H: FnMut() -> R,
   E: ParseError<I>,
-  R: Clone,
 {
   move |mut input: I| {
-    let mut acc = init.clone();
+    let mut acc = init();
     for count in 0..max {
       match parse.parse(input.clone()) {
         Ok((tail, value)) => {


Test patch:
diff --git a/src/multi/tests.rs b/src/multi/tests.rs
index 0ce46d26d..2a96f845b 100644
--- a/src/multi/tests.rs
+++ b/src/multi/tests.rs
@@ -422,10 +422,10 @@ fn fold_many0_test() {
     acc
   }
   fn multi(i: &[u8]) -> IResult<&[u8], Vec<&[u8]>> {
-    fold_many0(tag("abcd"), Vec::new(), fold_into_vec)(i)
+    fold_many0(tag("abcd"), Vec::new, fold_into_vec)(i)
   }
   fn multi_empty(i: &[u8]) -> IResult<&[u8], Vec<&[u8]>> {
-    fold_many0(tag(""), Vec::new(), fold_into_vec)(i)
+    fold_many0(tag(""), Vec::new, fold_into_vec)(i)
   }
 
   assert_eq!(multi(&b"abcdef"[..]), Ok((&b"ef"[..], vec![&b"abcd"[..]])));
@@ -454,7 +454,7 @@ fn fold_many1_test() {
     acc
   }
   fn multi(i: &[u8]) -> IResult<&[u8], Vec<&[u8]>> {
-    fold_many1(tag("abcd"), Vec::new(), fold_into_vec)(i)
+    fold_many1(tag("abcd"), Vec::new, fold_into_vec)(i)
   }
 
   let a = &b"abcdef"[..];
@@ -481,7 +481,7 @@ fn fold_many_m_n_test() {
     acc
   }
   fn multi(i: &[u8]) -> IResult<&[u8], Vec<&[u8]>> {
-    fold_many_m_n(2, 4, tag("Abcd"), Vec::new(), fold_into_vec)(i)
+    fold_many_m_n(2, 4, tag("Abcd"), Vec::new, fold_into_vec)(i)
   }
 
   let a = &b"Abcdef"[..];
diff --git a/tests/arithmetic.rs b/tests/arithmetic.rs
index 393c7e767..79194a162 100644
--- a/tests/arithmetic.rs
+++ b/tests/arithmetic.rs
@@ -39,7 +39,7 @@ fn term(i: &str) -> IResult<&str, i64> {
 
   fold_many0(
     pair(alt((char('*'), char('/'))), factor),
-    init,
+    move || init,
     |acc, (op, val): (char, i64)| {
       if op == '*' {
         acc * val
@@ -55,7 +55,7 @@ fn expr(i: &str) -> IResult<&str, i64> {
 
   fold_many0(
     pair(alt((char('+'), char('-'))), term),
-    init,
+    move || init,
     |acc, (op, val): (char, i64)| {
       if op == '+' {
         acc + val
diff --git a/tests/json.rs b/tests/json.rs
index 5b30c74a8..404402bab 100644
--- a/tests/json.rs
+++ b/tests/json.rs
@@ -83,7 +83,7 @@ fn character(input: &str) -> IResult<&str, char> {
 fn string(input: &str) -> IResult<&str, String> {
   delimited(
     char('"'),
-    fold_many0(character, String::new(), |mut string, c| {
+    fold_many0(character, String::new, |mut string, c| {
       string.push(c);
       string
     }),
diff --git a/tests/reborrow_fold.rs b/tests/reborrow_fold.rs
index d356176e3..486617e42 100644
--- a/tests/reborrow_fold.rs
+++ b/tests/reborrow_fold.rs
@@ -23,7 +23,7 @@ fn atom<'a>(_tomb: &'a mut ()) -> impl FnMut(&'a [u8]) -> IResult<&'a [u8], Stri
 fn list<'a>(i: &'a [u8], tomb: &'a mut ()) -> IResult<&'a [u8], String> {
   delimited(
     char('('),
-    fold_many0(atom(tomb), String::new(), |acc: String, next: String| {
+    fold_many0(atom(tomb), String::new, |acc: String, next: String| {
       acc + next.as_str()
     }),
     char(')'),