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

Parse pinned local variable declarations #135631

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frank-king
Copy link
Contributor

This PR implements part of #130494, parsing let pin mut x or let pin const x in local variables that cannot be moved.

@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2025

r? @Noratrieb

rustbot has assigned @Noratrieb.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 17, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 17, 2025

Some changes occurred in match checking

cc @Nadrieril

@rust-log-analyzer

This comment has been minimized.

@Noratrieb
Copy link
Member

The tracking issue only mentions constructors and reborrowing, and not pinned locals, so I don't think this should be added.
I don't even know what this should do, it is very non-obvious to me. Can you explain to me what you'd expect this to do?

@frank-king
Copy link
Contributor Author

frank-king commented Jan 18, 2025

Compared to mut locals, pin mut locals can be borrowed by &pin mut, but cannot be moved by assignment, passed to a function, or mutable borrow &mut.

Similarly, pin const locals cannot be passed to a function, or something like Cell::take.

For example:

struct Foo;

impl Foo {
    fn by_pin_const(&pin const self) {}
    fn by_pin_mut(&pin mut self) {}
    fn by_ref(&self) {}
    fn by_mut(&mut self) {}
    fn by_value(self) {}
}

fn bar() {
    let pin mut foo = Foo; // define a pinned mutable local (cannot be moved but can be mutated)
    foo.by_pin(); // ok
    foo.by_pin_mut(); // ok
    foo.by_ref(); // ok because `Pin<Foo>` impls Deref<Target = Foo>
    foo.by_mut(); //~ ERROR cannot borrow `foo` mutably because it is pinned
    foo.by_value();  //~ ERROR cannot move `foo` because it is pinned
    foo = Foo; //~ ERROR cannot assign to `foo` because the previous value it bound has be pinned

    let pin const foo = Foo; // define a pinned immutable local (cannot be moved nor mutated)
    foo.by_pin(); // ok
    foo.by_pin_mut(); // ERROR cannot borrow `foo` mutably because it is immutably pinned
    foo.by_ref(); // ok because `Pin<Foo>` impls Deref<Target = Foo>
    foo.by_mut(); //~ ERROR cannot borrow `foo` mutably because it is immutably pinned
    foo.by_value();  //~ ERROR cannot move `foo` because it is pinned
    foo = Foo; //~ ERROR cannot assign to `foo` because the `foo` is immuable
}

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature needs to be gated properly so that we reject this in #[cfg]'d out code.

@frank-king frank-king force-pushed the feature/pinned-local branch from e311c1e to 87de2da Compare January 20, 2025 05:31
@rustbot
Copy link
Collaborator

rustbot commented Jan 20, 2025

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@frank-king frank-king force-pushed the feature/pinned-local branch from 87de2da to 9d32ffd Compare January 20, 2025 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants