-
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
Add span label to E0384 for MIR borrowck #44806
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
r? @pnkfelix |
@KiChjang can you look into porting the test, in the same manner as discussed at #44811 (comment) |
src/librustc_mir/borrow_check.rs
Outdated
continue; | ||
} | ||
Lvalue::Local(local) => { | ||
let assigned_span = self.mir.local_decls[local].source_info.span; |
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.
Does the source info for the declaration always correspond to the assigned span when reported by AST borrowck?
In particular, I'm thinking of cases like:
let declare_without_init: i32;
match result {
Ok(_) => declare_without_init = 1,
Err(_) => declare_without_init = 2,
}
...
declare_without_init = 3;
where the AST-borrowck today, for better or worse, tags the assignment in the first match arm as the "first assignment"
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.
I don't actually know, I've only looked around the source code and used my own deduction skills to do what makes sense in order to retrieve a span. Perhaps @nikomatsakis may know more about this?
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.
I just compiled the example you gave with my patch -- it actually does the wrongest thing, which is displaying the span of let declare_without_init: i32;
. It would seem like mir.local_decls
would not provide the location in which a variable is assigned.
58c5692
to
3bfe4c0
Compare
I believe this can be re-reviewed. r? @pnkfelix |
I think your revision tags are not working:
|
@@ -8,6 +8,9 @@ | |||
// option. This file may not be copied, modified, or distributed | |||
// except according to those terms. | |||
|
|||
// revisions: ast mir | |||
//[mir]compfile-flags: -Zemit-end-regions -Zborrowck-mir |
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.
I think you want to use [mir]compile-flags
(not compfile) here. I think that's why you're getting the travis error.
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.
Ack, this is so embarrassing!
9fcfad9
to
afa52ca
Compare
|
6354d75
to
c8ae9f4
Compare
c8ae9f4
to
6d4989b
Compare
@bors r+ |
📌 Commit 6d4989b has been approved by |
Add span label to E0384 for MIR borrowck Corresponds to `report_illegal_reassignment`. Part of #44596.
☀️ Test successful - status-appveyor, status-travis |
Corresponds to
report_illegal_reassignment
.Part of #44596.