Skip to content

Tracking issue for generalized two-phase borrows #49434

@sapphire-arches

Description

@sapphire-arches
Contributor

This can be sort-of considered a tracking issue for the implementation of rust-lang/rfcs#2025

Right now, two phase borrow support can only handle the very simple case where there is exactly one activation point. In particular, this means we can't expose two-phase borrows to user code since the following wouldn't work:

let x = &mut some_val;
if some_cond {
   *x = 1;
} else {
   *x = 2;
}

We also intentionally limit two-phase borrows to a very narrow subset of its possible applications; specifically cases where the compiler is injecting mutable auto borrows which are used as method/function parameters. In practice, this covers the following cases:

  • using x.some_method() syntax, where some_method takes &mut self
  • using Foo::some_method(&mut x, ...) syntax
  • binary assignment operators (+=, -=, *=, etc.) Note this does /not/ include IndexMut operations at this time

Basically, things that desugar to method calls with a mutable self parameter with the exception of IndexMut. The intention with these restrictions is leverage two-phase borrows to make MIR borrowck accept AST borrowck can, without introducing too much new flexibility (ref #48431 for a more detailed discussion of how and why this came to be). The eventual intention (per rust-lang/rfcs#2025) is to extend this to the general case outlined above, which is what this issue tracks.

Activity

pnkfelix

pnkfelix commented on Mar 28, 2018

@pnkfelix
Member

I've become less convinced that we will actually want generalized two-phase borrows, as I commented earlier, (quoted here for convenient reference:

I actually don't think you/we should worry about trying to support 2-phase borrow beyond autoref.

I had originally thought the general idea could make sense, but at this point I think there are enough cases where it does not work that it would not be a good use of time to try to hack it together. Or at least, I'd want to see a more theoretically (sic) proof of soundness for the idea before we implemented anything.

added
C-enhancementCategory: An issue proposing an enhancement or a PR with one.
C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
on Apr 24, 2018
KiChjang

KiChjang commented on Oct 11, 2018

@KiChjang
Member

Here is a sample code that highlights the deficiency of the current formulation:

fn main() {
    let mut x = vec![];
    x.push(x.len());
    x.get_mut(x.len());
    println!("Hello, world!");
}

(playground)

which results in

error[E0502]: cannot borrow `x` as immutable because it is also borrowed as mutable
 --> src/main.rs:4:15
  |
4 |     x.get_mut(x.len());
  |     - ------- ^ immutable borrow occurs here
  |     | |
  |     | mutable borrow later used by call
  |     mutable borrow occurs here

error: aborting due to previous error

This is baffling because the x.push(x.len()); statement did not throw an error, but somehow dereffing and calling Slice::get_mut causes the mutable borrow to be activated prematurely.

pnkfelix

pnkfelix commented on Dec 19, 2018

@pnkfelix
Member

Hmm. I think this ties into broader questions of the semantics of two-phase borrows (see #56254) and how we document it (#46901).

pnkfelix

pnkfelix commented on Dec 19, 2018

@pnkfelix
Member

In terms of re-triaging for #56754, I think this is NLL-complete, P-medium.

added
NLL-completeWorking towards the "valid code works" goal
and removed on Dec 19, 2018
bluescreen303

bluescreen303 commented on Mar 12, 2019

@bluescreen303

I ran into a very similar issue, borrow-after-move, where a generalized two-phase solution would help and would make behaviours more as expected.

Please see https://stackoverflow.com/questions/55117392/am-i-misunderstanding-evaluation-order-borrow-of-moved-value

jeapostrophe

jeapostrophe commented on Sep 23, 2020

@jeapostrophe
pnkfelix

pnkfelix commented on May 27, 2022

@pnkfelix
Member

Visting for T-compiler backlog bonanza.

I'm still of the opinion that I don't want us to invest too much effort in generalizing two-phase borrows without more formal proof of its soundness.

The new T-types team is in the best spot to produce that.

So, moving this to T-types and adding S-tracking-design-concerns:

@rustbot label: -T-compiler +T-types +S-tracking-design-concerns

added
T-typesRelevant to the types team, which will review and decide on the PR/issue.
and removed
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
on May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-NLLArea: Non-lexical lifetimes (NLL)C-enhancementCategory: An issue proposing an enhancement or a PR with one.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCNLL-completeWorking towards the "valid code works" goalP-mediumMedium priorityS-tracking-design-concernsStatus: There are blocking design concerns.T-typesRelevant to the types team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @bluescreen303@jkordish@nikomatsakis@pnkfelix@jeapostrophe

        Issue actions

          Tracking issue for generalized two-phase borrows · Issue #49434 · rust-lang/rust