Implement borrowck for &pin mut|const $place

[frank-king]

View all comments

Part of Pin Ergonomics.

This forbids places to be moved or mutably borrowed ever since they are pinned until they are reassigned (even after the pinned borrows themselves expire).

#![feature(pin_ergnonmics)]
#![allow(incomplete_features)]

#[pin_v2]
struct Foo;

fn foo(mut x: Foo) {
    {
        let _x = &pin mut x; // x is pinned
    } // now the pinned reference expires
    let _x = &mut x; //~ ERROR cannot borrow `x` as mutable because it is pinned
    let _x = x; //~ ERROR cannot move out of `x` because it is pinned
    
    x = Foo; // x is reassigned
    let _x = &mut x; // ok because x is reassigned and not pinned yet
}

Tasks:

• Add Pins dataflow analysis
• Improve the diagnostics of pinning errors
• Test pin pattern matching
• Forbid &pin borrows for ADTs without #[pin_v2] (for soundness concerns)

Unresolved questions

When should #[pin_v2] be checked?

Currently, it is checked at rustc_hir_typck::FnCtxt::check_expr_addr_of, but field projections are not correctly handled.

Should we check this after the expr adjustments are available or even at THIR?

Related discussions:

• https://github.com/rust-lang/rust/pull/153693/changes#r3212760135
• Require #[pin_v2] for direct &pin borrows of ADTs frank-king/rust#2 (comment)

How to deal with Unpin types?

Should &pin borrowed places of Unpin types be prevented from being moved, or just don't treat Unpin types specially and emit move-after-pin as usual?

[frank-king]

Let me self-review the whole PR carefully before marking it ready for review.

[P8L1]

@frank-king How is your review going?

[frank-king]

This is probably wrong. Generally, let x = &pin mut y; should be equivalent to let ref mut x = y;. Given that (Playground)

struct NoPinV2 {
    x: i32,
}

struct NoPinV2NoUnpin {
    x: i32,
    __: std::marker::PhantomPinned,
}


struct NoPinV2Generic<T> {
    y: T,
}

struct NoPinV2GenericUnpin<T> {
    y: T,
    __: std::marker::PhantomPinned,
}

impl<T> Unpin for NoPinV2GenericUnpin<T> {}

fn foo<T>(
    mut x: NoPinV2,
    mut y: NoPinV2NoUnpin,
    mut z: NoPinV2Generic<T>,
    mut w: NoPinV2GenericUnpin<T>,
) {
    let ref pin mut x = x; // ok
    let ref pin mut y = y; // ok
    let ref pin mut z = z; // ok
    let ref pin mut w = w; // ok
    // However, only the first is accepted
    let x = &pin mut x; // ok
    let y = &pin mut y; // ERROR
    let z = &pin mut z; // ERROR
    let w = &pin mut w; // ERROR
}

Regarding pin patterns, the #[pin_v2] checks actually happen when field projections take place, rather than when pinned borrows (via the ref pin patterns) do.

I'm afraid that we should check #[pin_v2] at field projections (including nested field projections like &pin mut foo.a[2].0) too. I guess this can be processed when walking at the expr adjuestments (but highly unsure)

View changes since the review

[rustbot]

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred to constck

cc @fee1-dead

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

[rustbot]

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 17 candidates

[frank-king]

I'm going to mark this PR ready since there isn't a simple and clear solution to the unresolved questions yet. Help wanted from reviewers.

CC @traviscross @oli-obk

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.