search

rust-bakery / nom

Rust parser combinator framework
MIT License
9.18k stars 792 forks source link

fold parsers still require cloning #1656

Closed djmitche closed 1 year ago

djmitche commented 1 year ago

https://github.com/rust-bakery/nom/pull/1354 removed the Clone bounds on R for fold parser, but replaced it with a FnMut bounds on the supplier function. The result is that, if your parser's accumulator begins with a non-empty value that's calculated in advance, you end up having to clone the value anyway:

let initial_value = some_sub_parser(input);
fold_many0(another_sub_parser, move || initial_value.clone(), |acc, item| { .. });

It seems that fold_many0 only calls init once, so it's not clear why this has to be FnMut and not just FnOnce?

djmitche commented 1 year ago

Hm, it appears that FnOnce is not possible since this is embedded in the FnMut returned by fold_many*, which may, in fact, be called multiple times in the case of backtracking.

In which case, the Clone bounds was somewhat accurate: this initial value may need to be created multiple times. The supplier function is certainly more flexible! What confused me is that "rewrite init to move || init" isn't quite accurate -- it assumes init is Copy. More accurate is rewriteinittomove || init.clone()`.