Improve move related error messages #47093
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @michaelwoerister (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. |
|
r? @arielb1 |
vramana
force-pushed
the
fix-46599
branch
2 times, most recently
from
c3ad3ac to
6ce408c
Compare
vramana
changed the title
| @@ -8,18 +8,22 @@ | |||
| // option. This file may not be copied, modified, or distributed | |||
| // except according to those terms. | |||
|
|
|||
| // revisions: ast mir | |||
| //[mir]compile-flags: -Z borrowck=mir | |||
|
|
|||
| struct FancyNum { | |||
| num: u8, | |||
| } | |||
|
|
|||
| fn main() { | |||
There was a problem hiding this comment.
While you're at this, could we move this file to test/ui?
| struct FancyNum { | ||
| num: u8, | ||
| } | ||
|
|
||
| fn main() { | ||
| let fancy_num = FancyNum { num: 5 }; | ||
| let mut fancy_num = FancyNum { num: 5 }; | ||
| let fancy_ref = &fancy_num; | ||
|
|
||
| let x = move || { |
There was a problem hiding this comment.
Could we add a secondary span label pointing at move || saying "moved into this closure"?
| // If the move occurs due to a closure, then we want to show the span | ||
| // of the place where it's used in the closure. | ||
| let closure_span = this.find_closure_span(place_span.1, context.loc); | ||
| if let Some((_, var_span)) = closure_span { |
There was a problem hiding this comment.
The first span is the args_span, you could use it to add a secondary span label pointing at move || saying "moved into this closure". That way there's no ambiguity (think of the case of nested closures).
| let closure_span = this.find_closure_span(place_span.1, context.loc); | ||
| if let Some((_, var_span)) = closure_span { | ||
| let place_span = (place_span.0, var_span); | ||
| this.report_move_into_closure(context, place_span, &borrow); |
There was a problem hiding this comment.
report_move_into_closure could be called from report_move_out_while_borrowed as it already has the context and place_span, the only things needed to verify wether it happened to be a closure move.
kennytm
added
the
S-waiting-on-review
|
r? @estebank |
carols10cents
added
S-waiting-on-author
|
Hi @vramana! Would you like to fix the remaining nit picks in #47093 (review)? Thanks! |
|
@kennytm Yes. I was a little busy the past week or so. I will fix the nits soon. |
kennytm
added
S-waiting-on-author
|
Hi @vramana, could you address #47093 (review) so the PR can be merged? |
|
Thank you for your hard work, @vramana ! Unfortunately, we haven't head from you in a few weeks, so we are going to go ahead and close this PR to keep the queue tidy. Please feel free to address the feedback and reopen this PR! |
shepmaster
added
S-inactive
Fixes #46599