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

Adapt pin! to the new lifespan-extension rules of 2024 edition #138622

Closed

Conversation

danielhenrymantilla
Copy link
Contributor

@danielhenrymantilla danielhenrymantilla commented Mar 17, 2025

Fixes #138596

See the docs of the lifetime_extension module for info about the implementation and the, alas, hacks involved.

The hackiest part is the need to add a PhantomData field to Pin, alongside dead code, to guide type inference so as to force a sorely needed DerefMut coërcion to happen.

Therefore, let's keep in mind possible:

Alternatives to this PR

In order, imho, of simplicity-and-shipping-velocity:

  1. Language tweak: get explicit Deref{,Mut}, i.e., &* and &mut *, to preserve lifespan-of-temps extension.

    Indeed, it is not currently the case, whereas implicit Deref{,Mut} (through deref-coërcions from type constraints) does preserve it!

    In other words, currently, we have the following silly disparity:

    use ::std::borrow::Cow;
    let b: &String = &Cow::Borrowed { 0: &String::from("…") } /* as &*… */;
    let _some_use = (&b, ); // OK
    // vs.
    let b /*: &String*/ = &*Cow::Borrowed { 0: &String::from("…") };
    let _some_use = (&b, ); // Error!

    With this, the implementation of this PR gets significantly reduced, insofar anything __phantom related disappears, and we'd just have:

    Pin { __pointer: &mut *__Pinned { value: $value } }
  2. Lib/compiler hack: expose a way for the pin! macro (or a helper macro thereof) to emit code using 2021 edition (for its braces) so that the emitted code benefits from the 2021 edition rules for lifespan-of-temps extension.

    What I dislike about this: it becomes an utter acknowledgment of edition 2024 having regressed in expressibility; we ought rather to try and find ways to be able to express 2021 edition semantics without actually needing to be using that edition, I'd say.

  3. Language feature (and proper solution): have the language offer super let.

    This is the proper tool through which the pin! macro and the like ought to be implemented:

    macro_rules! pin {( $value:expr $(,)? ) => ({
     // extends the life-span of this `anon` local.
     // vvvvv
        super let mut anon = $value;
        // No need for field privacy hacks, or brittle braced constructions.
        // For instance, anybody could implement this, not just the stdlib.
        unsafe { Pin::new_unchecked(&mut anon) }
    })}

    The problem is that I don't see this happening quick enough w.r.t. fixing issues such as pin macro no longer lifetime extends argument #138596 (unless we temporarily reverted the edition of core back to 2021 in the meantime?).

@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2025

r? @joboet

rustbot has assigned @joboet.
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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 17, 2025
@rust-log-analyzer

This comment has been minimized.

@theemathas
Copy link
Contributor

Code like let x = pin!(&mut returns_a_future());, which people apparently do write in practice, previously used to work. I believe that your fix still causes this code to fail.

@danielhenrymantilla
Copy link
Contributor Author

danielhenrymantilla commented Mar 18, 2025

Huh, preserving the exact semantics of 2021 edition has proven to be way more challenging than anticipated, albeit doable at the cost of more hacks (that is, the newer implementation in this PR fixes both the match-scrutinee issue without regressing for the pin!(&mut …) occurrences that @theemathas has pointed out).

Maybe it will be better to "just" imbue the pin! macro (or a helper macro thereof) with 2021 edition?

r? @lcnr

@rustbot rustbot assigned lcnr and unassigned joboet Mar 18, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] vfs_notify test:true 0.136
error[E0282]: type annotations needed for `std::pin::Pin<&mut _>`
   --> src/tools/rust-analyzer/crates/hir-ty/src/layout/tests.rs:395:17
    |
395 |             let pinned = pin!(inp);
    |                 ^^^^^^
...
404 |             pinned.poll(&mut context)
    |                    ---- type must be known at this point
    |
help: consider giving `pinned` an explicit type, where the placeholders `_` are specified
    |
395 |             let pinned: std::pin::Pin<&mut _> = pin!(inp);
    |                       +++++++++++++++++++++++

[RUSTC-TIMING] project_model test:true 1.147
[RUSTC-TIMING] rust_analyzer test:false 5.868
[RUSTC-TIMING] slow_tests test:true 0.436

@danielhenrymantilla danielhenrymantilla marked this pull request as draft March 18, 2025 18:04
@danielhenrymantilla
Copy link
Contributor Author

danielhenrymantilla commented Mar 18, 2025

Hmm, the unsized coërcion is giving me some trouble, it's non-trivial.

@rustbot label -S-waiting-on-review +S-waiting-on-author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2025
@traviscross
Copy link
Contributor

cc @dingxiangfei2009 @nikomatsakis

Comment on lines +2069 to +2072
/// The reason for that is that we'd end up with `{ foo(&mut temporary(0)) }` among the expansion
/// of `pin!`, and since this happens in `match`-scrutinee position, temporaries get not to outlive
/// their encompassing braced block(s), so `temporary(0)` dies before that `}`, and the resulting
/// `Pin { … }` is born dangling.
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't matter about this being in match scrutinee position. E.g., this also fails in Rust 2024 and works in Rust 2021:

fn f<'a, T>(x: &'a T) -> &'a T { x }
fn g<T>(_: T) {}

struct W<T: ?Sized> {
    _inner: T,
}

fn main() {
    _ = g(W { _inner: &mut { f(&mut ()) } });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll edit if we keep up with this PR

@danielhenrymantilla
Copy link
Contributor Author

Alas, I'll have to abandon this effort 😕. I thought I'd be able to use enough workarounds to produce the necessary typeof<$value> semantics to force an implicit DerefMut coërcion, but without the ability to use any kind of blocks whatsoever (since those immediately kill lifespan-of-temps extension as per the 2024 edition rules —the more I experiment with this, the more I find the change rather extreme, we truly lost expressibility of the language—), it's just not possible without repeating the $value argument of the caller.

  • I had initially thought that not to be that much of a problem by using unreachable_code that would disable move semantics and borrow-checking, but it's actually straight up broken then moment $value contains on-the-fly-generation-of-new-types, such as closures, or async blocks (e.g., when $value = `async {}`, then typeof<$value> ≠ typeof<$value>).

We'll need one of the "Alternatives to this PR" in order to showcase a solution

@m-ou-se
Copy link
Member

m-ou-se commented Mar 19, 2025

Language feature (and proper solution): have the language offer super let.
This is the proper tool through which the pin! macro and the like ought to be implemented:
rust macro_rules! pin {( $value:expr $(,)? ) => ({ // extends the life-span of this `anon` local. // vvvvv super let mut anon = $value; // No need for field privacy hacks, or brittle braced constructions. // For instance, anybody could implement this, not just the stdlib. unsafe { Pin::new_unchecked(&mut anon) } })}

It's good to have more motivation to add super let. But yeah, that won't exist tomorrow or next week. It could be the longer term solution, if we can find a quick hack to fix it for now.

@m-ou-se
Copy link
Member

m-ou-se commented Mar 19, 2025

a quick hack to fix it for now

We have a quick fix: #138717

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pin macro no longer lifetime extends argument
8 participants