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

MIR borrowck: move span_label to borrowck_errors.rs #44922

Merged
merged 3 commits into from
Oct 3, 2017

Conversation

zilbuz
Copy link
Contributor

@zilbuz zilbuz commented Sep 29, 2017

The calls to span_label are moved and factorized for:

  • E0503 (cannot_use_when_mutably_borrowed())
  • E0506 (cannot_assign_to_borrowed())

Additionnally, the error E0594 (cannot_assign_static()) has been factorized between check_loan.rs and borrowc_check.rs.

Part of #44596

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@shepmaster shepmaster added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2017
@@ -57,12 +60,18 @@ fn main() {
let mut s = "hello".to_string();
let rs = &mut s;
println!("{}", f[&s]);
//~^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable
//[ast]~^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable
//[mir]~^^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable (Ast)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use ~| here instead of ~^^

f[&s] = 10;
//~^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable
//[ast]~^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable
//[mir]~^^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable (Ast)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use ~| here instead of ~^^

_iter.node = & //~ ERROR cannot assign to immutable field
_iter.node = & //[ast]~ ERROR cannot assign to immutable field
//[mir]~^ ERROR cannot assign to immutable field `_iter.node` (Ast)
// FIXME Error for MIR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh looks like a new mir-borrowck unsoundness bug we need to file

let s = Bar {
x: 1,
};
s[2] = 20;
//~^ ERROR cannot assign to immutable indexed content
//[ast]~^ ERROR cannot assign to immutable indexed content
//[mir]~^^ ERROR cannot assign to immutable indexed content
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no "(Ast)" suffix emitted here? If so, then it might be the case that the error being generated here is not the responsibility of mir-borrowck. (This is part of a broad category of things that need investigation, the cases where the emitted error is missing its "(Origin)" annotation and thus it is unclear which component is actually detecting the error when we run under -Z borrowck-mir.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 2, 2017

The refactoring looks fine to me. I had some nits that I noted above, but I don't think any of them are worth blocking landing this PR.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 2, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Oct 2, 2017

📌 Commit d328d26 has been approved by pnkfelix

@bors
Copy link
Contributor

bors commented Oct 2, 2017

⌛ Testing commit d328d26 with merge 5bcfe12...

bors added a commit that referenced this pull request Oct 2, 2017
MIR borrowck: move span_label to `borrowck_errors.rs`

The calls to `span_label` are moved and factorized for:
* E0503 (`cannot_use_when_mutably_borrowed()`)
* E0506 (`cannot_assign_to_borrowed()`)

Additionnally, the error E0594 (`cannot_assign_static()`) has been factorized between `check_loan.rs` and `borrowc_check.rs`.

Part of #44596
@bors
Copy link
Contributor

bors commented Oct 2, 2017

💔 Test failed - status-appveyor

@pnkfelix
Copy link
Member

pnkfelix commented Oct 3, 2017

@bors retry

For the record @nagisa has previously pointed out that this may be evidence of a spurious test failure, see also #44906 (comment)

@nikomatsakis
Copy link
Contributor

r? @pnkfelix

@pnkfelix pnkfelix added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2017
@aidanhs
Copy link
Member

aidanhs commented Oct 3, 2017

Spurious failure issue: #43402

@pnkfelix
Copy link
Member

pnkfelix commented Oct 3, 2017

@bors retry

@bors
Copy link
Contributor

bors commented Oct 3, 2017

⌛ Testing commit d328d26 with merge 835e3e5...

bors added a commit that referenced this pull request Oct 3, 2017
MIR borrowck: move span_label to `borrowck_errors.rs`

The calls to `span_label` are moved and factorized for:
* E0503 (`cannot_use_when_mutably_borrowed()`)
* E0506 (`cannot_assign_to_borrowed()`)

Additionnally, the error E0594 (`cannot_assign_static()`) has been factorized between `check_loan.rs` and `borrowc_check.rs`.

Part of #44596
@bors
Copy link
Contributor

bors commented Oct 3, 2017

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

@bors bors merged commit d328d26 into rust-lang:master Oct 3, 2017
@zilbuz zilbuz deleted the issue-44596/E0594 branch October 4, 2017 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants