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

Indexing regression in beta #48715

Closed
Mark-Simulacrum opened this issue Mar 4, 2018 · 8 comments
Closed

Indexing regression in beta #48715

Mark-Simulacrum opened this issue Mar 4, 2018 · 8 comments
Labels
A-borrow-checker Area: The borrow checker C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

seal-0.1.0 regressed with the following:

error[E0502]: cannot borrow `*self` as immutable because it is also borrowed as mutable
  --> src/pair/alignment_matrix.rs:79:26
   |
79 |         self.mut_slice()[self.offset(cursor)] = byte;
   |         ----             ^^^^               - mutable borrow ends here
   |         |                |
   |         |                immutable borrow occurs here
   |         mutable borrow occurs here

error: aborting due to previous error

error: Could not compile `seal`.

This looks vaguely familiar but looking at pull requests didn't seem to find anything apparent.

cc @rust-lang/compiler

@Mark-Simulacrum Mark-Simulacrum added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Mar 4, 2018
@Mark-Simulacrum Mark-Simulacrum added this to the 1.25 milestone Mar 4, 2018
@Mark-Simulacrum
Copy link
Member Author

cc @regexident

@nikomatsakis
Copy link
Contributor

I too remember something like this. I think it was a bug fix. Let me see if I can find it.

@nikomatsakis
Copy link
Contributor

Maybe same problem as reported here: #47349

@kennytm kennytm added A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Mar 4, 2018
@Mark-Simulacrum
Copy link
Member Author

Okay, we should investigate. I'm going to nominate and set to P-high for compiler team eyes, if it's the same reason as #47349 then let's just close -- I don't think we need to do much more than that if it's a bugfix.

@kennytm
Copy link
Member

kennytm commented Mar 4, 2018

The regression is caused by #47167 according to bisection, as expected.

Test program:

#![crate_type = "rlib"]
struct Cursor;
struct AlignmentMatrix(Vec<u8>);
impl AlignmentMatrix {
    fn offset(&self, _: &Cursor) -> usize {
        0
    }
    fn mut_slice(&mut self) -> &mut [u8] {
        &mut self.0
    }
    pub fn set_at(&mut self, cursor: &Cursor) {
        self.mut_slice()[self.offset(cursor)] = 0;
    }
}

Test script:

#!/bin/sh
$RUSTC_RELATIVE -V
! $RUSTC_RELATIVE 1.rs                  

Result:

$ RUST_LOG=rust_sysroot=info target/release/bisect --start b65f0bedd2f22d9661ecb7092f07746dc2ccfb0d --end c83fa5d91c3b16459ab7b87c48ed18bd059a23af --test test.sh

INFO:rust_sysroot: Getting commits from the git checkout in b65f0bedd2f22d9661ecb7092f07746dc2ccfb0d...c83fa5d91c3b16459ab7b87c48ed18bd059a23af
INFO:rust_sysroot: Received 159 commits
Searching in 159 commits; about 8 steps
rustc 1.25.0-nightly (97520ccb1 2018-01-21)
rustc 1.25.0-nightly (27ede5541 2018-01-10)
rustc 1.25.0-nightly (bb345a0be 2018-01-15)
rustc 1.25.0-nightly (9b2f8ac29 2018-01-13)
rustc 1.25.0-nightly (0b90e4e8c 2018-01-12)
rustc 1.25.0-nightly (619ced057 2018-01-11)
rustc 1.25.0-nightly (c9c298073 2018-01-11)
rustc 1.25.0-nightly (f62f77403 2018-01-10)
searched commits b65f0bedd2f22d9661ecb7092f07746dc2ccfb0d through c83fa5d91c3b16459ab7b87c48ed18bd059a23af
regression in 40; Some(Commit { sha: "f62f774035735a06c880c48c0b9017fcc0577e33", date: 2018-01-10T12:29:05Z, summary: "Auto merge of #47167 - ivanbakel:builtin_indexing, r=nikomatsakis" })

@regexident
Copy link
Contributor

So, what did I win? 😅

(Joking aside, any way I can be of help here?)

@kennytm
Copy link
Member

kennytm commented Mar 5, 2018

@regexident You could split that into two statements:

let offset = self.offset(cursor);
self.mut_slice()[offset] = byte;

@nikomatsakis
Copy link
Contributor

Since this is due to a confirmed bug fix, I'm going to close the issue then. @regexident please let us know though if we can help in any other way!

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 C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants