-
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
Fix incorrect span in &mut
suggestion
#49931
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
r? @estebank |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
f962bb6
to
bb2e1b4
Compare
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
Thanks for the fix!
Small changes needed before being able to merge it. Once they are addressed, r=me.
src/librustc_mir/borrow_check/mod.rs
Outdated
@@ -1654,7 +1662,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { | |||
|
|||
if let Some((err_help_span, err_help_stmt, item_msg, sec_span)) = err_info { | |||
let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir); | |||
err.span_suggestion(err_help_span, err_help_stmt, format!("")); | |||
err.span_suggestion(err_help_span, &err_help_stmt, format!("")); |
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.
If you look below, the third argument is the code content that will be suggested. Because here an empty String
is being passed, the suggestion effectively tells the user to remove the code.
src/librustc_mir/borrow_check/mod.rs
Outdated
self.tcx.sess.codemap().span_to_snippet(sp) { | ||
help_stmt = format!( | ||
"consider changing this to be a \ | ||
mutable reference: `&mut {}`", &src[1..]); |
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 help message doesn't need to incorporate the suggested code, this is passed as the third argument to span_suggestion
and the diagnostic machinery deals with this as appropriate.
src/librustc_mir/borrow_check/mod.rs
Outdated
help_stmt = format!( | ||
"consider changing this to be a \ | ||
mutable reference: `&mut {}`", &src[1..]); | ||
}; | ||
err_info = Some(( |
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.
err_info
will need to include the contents of &src[1..]
above in order to supply to use it in the suggestion.
src/test/ui/nll/issue-47388.stderr
Outdated
LL | fancy_ref.num = 6; //~ ERROR E0594 | ||
| ^^^^^^^^^^^^^^^^^ `fancy_ref` is a `&` reference, so the data it refers to cannot be written | ||
help: consider changing this to be a mutable reference: `&mut (&mut fancy)` | ||
| | ||
LL | let fancy_ref = ; |
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.
You can see what I'm referring to in the previous comments here: let fancy_ref = ;
should be
help: consider changing this to be a mutable reference
|
LL | let fancy_ref = &mut (&mut fancy);
span_suggestion
deals with this correctly automatically.
Advice addressed. r @estebank |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r+ rollup |
📌 Commit 6f5a16b has been approved by |
Fix incorrect span in `&mut` suggestion Fixes rust-lang#49859
Rollup of 8 pull requests Successful merges: - #49555 (Inline most of the code paths for conversions with boxed slices) - #49606 (Prevent broken pipes causing ICEs) - #49646 (Use box syntax instead of Box::new in Mutex::remutex on Windows) - #49647 (Remove `underscore_lifetimes` and `match_default_bindings` from active feature list) - #49931 (Fix incorrect span in `&mut` suggestion) - #49959 (rustbuild: allow building tools with debuginfo) - #49965 (Remove warning about f64->f32 cast being potential UB) - #49994 (Remove unnecessary indentation in rustdoc book codeblock.) Failed merges:
Fixes #49859