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

Arithmetic makes borrow checker overly restrictive #29743

Closed
dotdash opened this issue Nov 10, 2015 · 14 comments
Closed

Arithmetic makes borrow checker overly restrictive #29743

dotdash opened this issue Nov 10, 2015 · 14 comments
Labels
A-borrow-checker Area: The borrow checker A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@dotdash
Copy link
Contributor

dotdash commented Nov 10, 2015

UPDATE: This was fixed by #47167 but still needs a regression test to be added.


fn main() {
    let mut i = [1,2,3];
    i[i[0]] = 0; // Works
    i[i[0] - 1] = 0; // Is rejected
}

I had expect the - 1 not to make any difference, but the borrow checker complains with the following error:

<anon>:4:7: 4:11 error: cannot use `i[..]` because it was mutably borrowed
<anon>:4     i[i[0] - 1] = 0;
               ^~~~
<anon>:4:5: 4:6 note: borrow of `i` occurs here
<anon>:4     i[i[0] - 1] = 0;
@steveklabnik
Copy link
Member

@rust-lang/lang, is this behavior expected? I wonder if there's something weird going on here

@nikomatsakis
Copy link
Contributor

Does seem odd. I suspect that the borrow checker is (incorrectly) still locked into the days when indices were passed by reference, rather than by value.

@nikomatsakis
Copy link
Contributor

triage: P-medium

@rust-highfive rust-highfive added the P-medium Medium priority label Nov 12, 2015
@nikomatsakis nikomatsakis added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Nov 12, 2015
@nikomatsakis
Copy link
Contributor

Likely to be fixed by MIR, though.

@sanxiyn
Copy link
Member

sanxiyn commented Aug 4, 2016

Still reproduces.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 4, 2016

(Note: would be fixed by a MIR-based borrowck is what I meant, which is not yet done.)

@brson brson added I-wrong E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. labels Aug 4, 2016
@nikomatsakis nikomatsakis added the A-borrow-checker Area: The borrow checker label Aug 4, 2016
@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. and removed I-wrong labels Jul 24, 2017
@cyplo
Copy link
Contributor

cyplo commented Nov 1, 2017

Still seems to be a problem on rustc 1.23.0-nightly (8b22e70 2017-10-31) - not sure if expected ?

error[E0503]: cannot use `i[..]` because it was mutably borrowed
 --> main.rs:4:7
  |
4 |     i[i[0] - 1] = 0; // Is rejected
  |     - ^^^^ use of borrowed `i`
  |     |
  |     borrow of `i` occurs here

error: aborting due to previous error

@pnkfelix
Copy link
Member

Hmm MIR-borrowck does not seem to address this case, at least not out-of-the-box

@pnkfelix pnkfelix added A-NLL Area: Non-lexical lifetimes (NLL) WG-compiler-nll labels Dec 21, 2017
@nikomatsakis
Copy link
Contributor

The first question is to decide, I suppose, what the expected behavior is =)

@nikomatsakis
Copy link
Contributor

In other words, I think I would expect consistent behavior, but I could see how this might wind up as an error in both cases.

@nikomatsakis
Copy link
Contributor

Well, based on this play test case, we do not get consistent behavior.

@nikomatsakis
Copy link
Contributor

I wonder if #47167 will affect this, actually.

@nikomatsakis
Copy link
Contributor

Seems like #47167 fixes it.

@nikomatsakis
Copy link
Contributor

It now works on nightly. Marking as E-needstest. Removing from WG-compiler-nll, since this was not specific to NLL in any way.

@nikomatsakis nikomatsakis added E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. and removed WG-compiler-nll E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. labels Jan 18, 2018
@nikomatsakis nikomatsakis added WG-compiler-nll fixed-by-NLL Bugs fixed, but only when NLL is enabled. labels Jul 3, 2018
euclio added a commit to euclio/rust that referenced this issue Oct 4, 2018
bors added a commit that referenced this issue Oct 7, 2018
Add tests for some E-needstest issues

Fixes #28134.
Fixes #24338.
Fixes #29743.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-NLL Area: Non-lexical lifetimes (NLL) C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. fixed-by-NLL Bugs fixed, but only when NLL is enabled. P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants