-
Notifications
You must be signed in to change notification settings - Fork 13k
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 help on pattern guard #33260
add help on pattern guard #33260
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Thanks for the PR! However, I don't like the way you did. :-/ |
I thought providing solution is enough. Yep u r right. To emphasize the two ways, it's better to rewrite the whole explanation. |
You can't unfortunately. I see two solutions to avoid this issue:
If someone else has something in mind, I'd be glad to hear it! :) |
You forgot the third case: when you can't clone. ;) Otherwise that's it! |
Hmm, according to this rfc, I don't think there is any other solutions except use non-consume function when you can't clone? Like this I'm sorry I don't understand "Setting a boolean and then, depending on this value, consume the type." I think if I call a consuming function, then the ownership is moved, so it will cause errors... |
The goal is to move the consuming function call outside of the closure. But I think it's just too much precise to be used. Let's not consider it. |
Yep, it's a too-specific situation. Just the two examples are enough. |
I let you handle it then! :) |
3c17cbd
to
f6b06d3
Compare
/cc @GuillaumeGomez Hope for your advice! It's my first PR to rust! |
|
||
```compile_fail | ||
The problem above can be solve by using `ref` keyword. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem above can be solve by using
ref
keyword.
"The problem above can be solved by using the ref
keyword."
Except for the nits, it seems all good. Great job! Ping me once corrected and squashed. |
f6b06d3
to
6f250d9
Compare
/cc @GuillaumeGomez |
A line of your commit is longer than 80 columns. If you want to check issues like this, you can use
I hope so as well! :) |
6f250d9
to
1528150
Compare
/cc @GuillaumeGomez |
reference when using guards or refactor the entire expression, perhaps by | ||
putting the condition inside the body of the arm. | ||
Though this example seems innocuous and easy to solve, the problem becomes clear | ||
when it encounters functions which consume the value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum... Finally, this sounds better with ":". Can you change "consume the value." to "consume the value:" please?
Sorry, one last nit and ready to get merged! |
1528150
to
665be00
Compare
|
fix too long column fix typo of help on pattern guard one nit fix compile fail
665be00
to
201d9ed
Compare
/cc @GuillaumeGomez @steveklabnik |
@bors: rollup |
@bors: r=guillaumegomez rollup |
📌 Commit 201d9ed has been approved by |
@mrmiywj sorry that this one slipped through the cracks! |
…illaumegomez add help on pattern guard
…illaumegomez add help on pattern guard
…illaumegomez add help on pattern guard
#33247