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

Closure captures immutable reference when mutable reference is necessary #88372

Closed
narpfel opened this issue Aug 26, 2021 · 3 comments · Fixed by #88466
Closed

Closure captures immutable reference when mutable reference is necessary #88372

narpfel opened this issue Aug 26, 2021 · 3 comments · Fixed by #88466
Labels
A-closures Area: closures (`|args| { .. }`) C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@narpfel
Copy link
Contributor

narpfel commented Aug 26, 2021

Code

I tried this code:

fn solve<F>(validate: F) -> Option<u64>
where
    F: Fn(&mut [i8; 1]),
{
    let mut position: [i8; 1] = [1];
    Some(0).map(|_| {
        validate(&mut position);
        let [_x] = position;
        0
    })
}

fn main() {
    solve(|_| ());
}

I expected to see this happen: The code should compile.

Instead, this happened: This compiler error is thrown:

$ rustc --version
rustc 1.56.0-nightly (0afc20860 2021-08-25)
$ rustc main.rs
error[E0596]: cannot borrow `position` as mutable, as it is behind a `&` reference
 --> main.rs:7:18
  |
7 |         validate(&mut position);
  |                  ^^^^^^^^^^^^^ cannot borrow as mutable

warning: variable does not need to be mutable
 --> main.rs:5:9
  |
5 |     let mut position: [i8; 1] = [1];
  |         ----^^^^^^^^
  |         |
  |         help: remove this `mut`
  |
  = note: `#[warn(unused_mut)]` on by default

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0596`.

Version it worked on

It most recently worked on: nightly-2021-07-11 (also works on stable 1.54). Starting from nightly-2021-07-12, I get the error shown above.

Version with regression

rustc --version --verbose: current nightly

$ rustc --version --verbose
rustc 1.56.0-nightly (0afc20860 2021-08-25)
binary: rustc
commit-hash: 0afc20860eb98a29d9bbeea80f2acc5be38c6bf3
commit-date: 2021-08-25
host: x86_64-unknown-linux-gnu
release: 1.56.0-nightly
LLVM version: 13.0.0

Also broken on 1.55 beta 7:

$ cargo +beta rustc -- --version --verbose
rustc 1.55.0-beta.7 (bf16ca353 2021-08-21)
binary: rustc
commit-hash: bf16ca353c82d9051dbd1a4cfc5a01a1800578e9
commit-date: 2021-08-21
host: x86_64-unknown-linux-gnu
release: 1.55.0-beta.7
LLVM version: 12.0.1

Both installed via rustup.

Backtrace

n/a


Additional information

This seems similar to #87814, which was fixed in #88266 / b03ccac (which AFAIU should be included in nightly-2021-08-25), but it still does not work.

@narpfel narpfel added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Aug 26, 2021
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Aug 26, 2021
@camelid camelid added A-closures Area: closures (`|args| { .. }`) regression-untriaged Untriaged performance or correctness regression. labels Aug 26, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Aug 26, 2021
@hellow554
Copy link
Contributor

Bisected to #86995

cc @arora-aman

@csmoe
Copy link
Member

csmoe commented Aug 27, 2021

type InferredCaptureInformation<'tcx> = FxIndexMap<Place<'tcx>, ty::CaptureInfo<'tcx>>;

the root cause is: if one place captured by more than 2 locations, the CaptureInfo will be overrided by the last one in processed.insert:
processed.insert(place, capture_info);

In this case, position is captured by MutBorrow at line 7 and ImmBorrow at line 8, the ImmBorrow captureinfo supercedes the previous MutBorrow.

@inquisitivecrystal inquisitivecrystal added regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Aug 28, 2021
@arora-aman
Copy link
Member

type InferredCaptureInformation<'tcx> = FxIndexMap<Place<'tcx>, ty::CaptureInfo<'tcx>>;

the root cause is: if one place captured by more than 2 locations, the CaptureInfo will be overrided by the last one in processed.insert:

processed.insert(place, capture_info);

In this case, position is captured by MutBorrow at line 7 and ImmBorrow at line 8, the ImmBorrow captureinfo supercedes the previous MutBorrow.

Thank you for this!

@bors bors closed this as completed in 5d68044 Aug 30, 2021
@camelid camelid removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: closures (`|args| { .. }`) C-bug Category: This is a bug. regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants