Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for mut bindings in function arguments #269

Open
djc opened this issue Jan 12, 2021 · 7 comments
Open

Add support for mut bindings in function arguments #269

djc opened this issue Jan 12, 2021 · 7 comments

Comments

@djc
Copy link

djc commented Jan 12, 2021

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
Copy link
Owner

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

@djc
Copy link
Author

djc commented Jan 12, 2021

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

@djc
Copy link
Author

djc commented Jan 12, 2021

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, 10XGenomics/rust-boomphf#19.)

@BurntSushi
Copy link
Owner

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
Copy link
Author

djc commented Jan 12, 2021

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

@djc
Copy link
Author

djc commented Jan 12, 2021

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
Copy link
Owner

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. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants