Skip to content
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

Merged
merged 1 commit into from
Sep 28, 2017

Conversation

KiChjang
Copy link
Member

Corresponds to report_illegal_reassignment.

Part of #44596.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 25, 2017
@KiChjang
Copy link
Member Author

r? @pnkfelix

@pnkfelix
Copy link
Member

@KiChjang can you look into porting the test, in the same manner as discussed at #44811 (comment)

continue;
}
Lvalue::Local(local) => {
let assigned_span = self.mir.local_decls[local].source_info.span;
Copy link
Member

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"

Copy link
Member Author

@KiChjang KiChjang Sep 25, 2017

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?

Copy link
Member Author

@KiChjang KiChjang Sep 26, 2017

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.

@KiChjang KiChjang force-pushed the mir-err-notes-2 branch 4 times, most recently from 58c5692 to 3bfe4c0 Compare September 26, 2017 09:55
@KiChjang
Copy link
Member Author

I believe this can be re-reviewed. r? @pnkfelix

@arielb1
Copy link
Contributor

arielb1 commented Sep 26, 2017

I think your revision tags are not working:


[00:51:10] ---- [compile-fail] compile-fail/borrowck/borrowck-match-binding-is-assignment.rs stdout ----
[00:51:10] 	
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:29: unexpected "error": '29:13: 29:19: re-assignment of immutable variable `x` [E0384]'
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:37: unexpected "error": '37:13: 37:19: re-assignment of immutable variable `x` [E0384]'
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:45: unexpected "error": '45:13: 45:19: re-assignment of immutable variable `x` [E0384]'
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:53: unexpected "error": '53:13: 53:19: re-assignment of immutable variable `x` [E0384]'
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:61: unexpected "error": '61:13: 61:19: re-assignment of immutable variable `x` [E0384]'
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:29: expected error not found: re-assignment of immutable variable `x` (Mir)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:29: expected error not found: re-assignment of immutable variable `x` (Ast)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:37: expected error not found: re-assignment of immutable variable `x` (Mir)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:37: expected error not found: re-assignment of immutable variable `x` (Ast)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:45: expected error not found: re-assignment of immutable variable `x` (Mir)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:45: expected error not found: re-assignment of immutable variable `x` (Ast)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:53: expected error not found: re-assignment of immutable variable `x` (Mir)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:53: expected error not found: re-assignment of immutable variable `x` (Ast)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:61: expected error not found: re-assignment of immutable variable `x` (Mir)
[00:51:10] 
[00:51:10] error in revision `mir`: /checkout/src/test/compile-fail/borrowck/borrowck-match-binding-is-assignment.rs:61: expected error not found: re-assignment of immutable variable `x` (Ast)
[00:51:10] 

@@ -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
Copy link
Contributor

@arielb1 arielb1 Sep 26, 2017

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.

Copy link
Member Author

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!

@KiChjang KiChjang force-pushed the mir-err-notes-2 branch 2 times, most recently from 9fcfad9 to afa52ca Compare September 26, 2017 18:34
@arielb1
Copy link
Contributor

arielb1 commented Sep 26, 2017

[00:14:00] error[E0308]: mismatched types
[00:14:00]    --> /checkout/src/librustc_mir/borrow_check.rs:594:75
[00:14:00]     |
[00:14:00] 594 |                 self.report_illegal_reassignment(context, (lvalue, span), assignment_span);
[00:14:00]     |                                                                           ^^^^^^^^^^^^^^^ expected struct `syntax_pos::Span`, found reference
[00:14:00]     |
[00:14:00]     = note: expected type `syntax_pos::Span`
[00:14:00]                found type `&rustc::mir::Statement<'_>`

@KiChjang KiChjang force-pushed the mir-err-notes-2 branch 2 times, most recently from 6354d75 to c8ae9f4 Compare September 26, 2017 21:27
@pnkfelix
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 27, 2017

📌 Commit 6d4989b has been approved by pnkfelix

@bors
Copy link
Contributor

bors commented Sep 28, 2017

⌛ Testing commit 6d4989b with merge d887369...

bors added a commit that referenced this pull request Sep 28, 2017
Add span label to E0384 for MIR borrowck

Corresponds to `report_illegal_reassignment`.

Part of #44596.
@bors
Copy link
Contributor

bors commented Sep 28, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing d887369 to master...

@bors bors merged commit 6d4989b into rust-lang:master Sep 28, 2017
@KiChjang KiChjang deleted the mir-err-notes-2 branch September 28, 2017 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants