BurntSushi / quickcheck

Automated property based testing for Rust (with shrinking).
The Unlicense
2.4k stars 149 forks source link

Add support for mut bindings in function arguments #269

Open djc opened 3 years ago

djc commented 3 years ago

I noticed that this works:

fn check_int_slices(v: HashSet<u64>, lens: Vec<usize>) -> bool {
    let mut lens = lens;
    ...
}

but this doesn't:

fn check_int_slices(v: HashSet<u64>, mut lens: Vec<usize>) -> bool {
    ...
}

Error:

error: no rules expected the token `lens`
   --> src/lib.rs:797:50
    |
797 |         fn check_int_slices(v: HashSet<u64>, mut lens: Vec<usize>) -> bool {
    |                                                  ^^^^ no rules expected this token in macro call
BurntSushi commented 3 years ago

Could you please provide a complete program? Mut bindings should have no impact on quickcheck.

djc commented 3 years ago

(Edited my example -- I just meant the mut binding for the function argument, plus the workaround for it.)

djc commented 3 years ago

It's here: https://github.com/10XGenomics/rust-boomphf/blob/master/src/lib.rs#L802

(Was looking at it because it started failing after upgrading from 0.9 to 0.10, https://github.com/10XGenomics/rust-boomphf/pull/19.)

BurntSushi commented 3 years ago

Okay, so you're talking about the macro. That wasn't at all clear.

And you're also reporting this as a regression? I don't think there were any changes to the macro.

I don't know when I'll be able to look into this, sorry. The quickcheck macro is not something that I understand well. A minimal reproduction would help.

djc commented 3 years ago

I'm not reporting this as a regression -- the breakage is unrelated to this bug report.

djc commented 3 years ago

I think the problem is basically in https://github.com/BurntSushi/quickcheck/blob/master/src/lib.rs#L48. The macro takes fn $fn_name:ident($($arg_name:ident : $arg_ty:ty),*) -> $ret:ty, which is constrained to only taking ident: ty for each argument, which doesn't cover the additional mut keyword.

BurntSushi commented 3 years ago

Okay, I'll mark this as an enhancement. My recollection is that the macro is quite gnarly, so anyone who likes playing in macro_rulea muck is invited to take a look at this. :)