-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Incorrect closure type deduced despite move keyword #44336
Comments
The workaround is to somehow make the move clearer. Either of the following work. But the compiler should be able to recognize this is a move without a workaround. - if let Some(s) = my_text {
+ if let Some(s) = {my_text} { - if let Some(s) = my_text {
+ let my_text1 = my_text;
+ if let Some(s) = my_text1 { |
An even simpler example that doesn't exhibit the expected outcome is:
This happily compiles and prints twice. It's as-if the |
I believe fn main() {
let opt = Some(vec![2]);
match opt {
Some(ref inner) => println!("{:?}", inner),
None => {},
}
println!("{:?}", opt); // no problem; the match only borrowed the contents
} @vitalyd 's example appears to be unrelated. The string is moved into the closure just fine (try using the string itself afterwards), but the closure implements fn main() {
let vec = vec![1,2,3];
let f = move || vec.len();
f(); // fine
f(); // fine (f implements FnMut)
vec.len(); // ERROR: value used after move (it belongs to the closure now)
}
On second thought, the more I think about it, the more that the error in the OP bothers me. |
Even so, we can clearly tell after the closing brace of the |
@ExpHP, yeah there are 2 slightly different issues/examples here. The example from rust users should not be a compiler error. The String example I gave shouldn't allow calling runner() twice. Both boil down to, I think, an incorrect type of closure to be determined by the compiler. Essentially, the move keyword doesn't do what it says on the tin :). |
Should also mention that it's important that move works here. Given Rust doesn't allow specifying capture lists nor capture style, users only have move to work with. In the Option example here, the binding used in the capture is the value itself. If someone wants to move a reference, then they should create a '&Option<...>' binding outside the closure and then use it inside the closure - then I'd expect the reference to move, which is of course just a copy of the reference. |
@vitalyd The |
@cuviper, that's one way to look at it, sure. The closure simply owns the String and you can call it multiple times. So how does one force FnOnce generation without syntactic hacks or extra verbiage? Perhaps this is a case where lack of explicit capture semantics is problematic. Also, I think the Option case is the more problematic one as that code should compile - I think that's what anyone looking at it would assume (unless they know of this gotcha). |
Writing the OP this way still fails: fn main() {
let my_text = Some(String::from("my_text"));
(move || {
if let Some(s) = my_text {
println!("Here it is: {}", s);
}
})();
} But adding indirection marked with fn call<F: FnOnce()>(f: F) { f() }
fn main() {
let my_text = Some(String::from("my_text"));
call(move || {
if let Some(s) = my_text {
println!("Here it is: {}", s);
}
});
} I would expect you could even drop the |
Yeah, this wouldn't be the only case where a workaround is needed in Rust. But, I'll admit that I would've thought the original code compiled without issue. It's also very newbie unfriendly given how vanilla the case is. |
Triage: OP's example now compiles on both editions. |
The fix appears to have been in |
This fails to compile on 1.20 (and a few earlier versions that I tried) with:
But this should be capturing
my_text
by move and generating anFnOnce
shouldn't it?This issue was originally brought up in https://users.rust-lang.org/t/moving-option-into-a-closure-by-binding-it-in-a-pattern-does-not-work-noob-question/12721
The text was updated successfully, but these errors were encountered: