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

pin macro no longer lifetime extends argument #138596

Closed
conradludgate opened this issue Mar 17, 2025 · 13 comments · Fixed by #138717
Closed

pin macro no longer lifetime extends argument #138596

conradludgate opened this issue Mar 17, 2025 · 13 comments · Fixed by #138717
Labels
A-edition-2024 Area: The 2024 edition A-pin Area: Pin C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. S-has-bisection Status: a bisection has been found for this issue T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@conradludgate
Copy link
Contributor

Code

I tried this code:

fn main() {
    match std::pin::pin!(foo(&mut 0)) {
        _f => {}
    }
}

async fn foo(_s: &mut usize) {}

I expected to see this happen: Compiles without error

Instead, this happened:

error[E0716]: temporary value dropped while borrowed
 --> src/main.rs:2:35
  |
2 |     match std::pin::pin!(foo(&mut 0)) {
  |           ------------------------^--
  |           |                       ||
  |           |                       |temporary value is freed at the end of this statement
  |           |                       creates a temporary value which is freed while still in use
  |           borrow later used here
  |
  = note: consider using a `let` binding to create a longer lived value

Version it worked on

It most recently worked on: 1.86.0-beta.6 (2025-03-16 8c7969a3ae4a292789a4)

Version with regression

rustc --version --verbose:

rustc 1.87.0-nightly (227690a25 2025-03-16)
binary: rustc
commit-hash: 227690a258492c84ae9927d18289208d0180e62f
commit-date: 2025-03-16
host: aarch64-apple-darwin
release: 1.87.0-nightly
LLVM version: 20.1.0
@conradludgate conradludgate added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Mar 17, 2025
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 17, 2025
@Nemo157
Copy link
Member

Nemo157 commented Mar 17, 2025

This seems very likely to be related to core migrating to edition 2024 (#138162). A manual expansion of the macro works when compiled with edition 2021 but fails on edition 2024 (playground).

@Noratrieb Noratrieb added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 17, 2025
@Noratrieb
Copy link
Member

Thank you for the report! It would be useful to bisect the regression using cargo-bisect-rustc to make it easier to figure out what happened and ping the relevant people.

@Fayti1703
Copy link
Contributor

Running a bisect with the OP code does indeed point the finger at the core edition change:


Regression in rust-lang-ci@6d75903
The PR introducing the regression in this rollup is #138162: Update the standard library to Rust 2024
searched nightlies: from nightly-2025-03-13 to nightly-2025-03-17
regressed nightly: nightly-2025-03-14
searched commit range: 249cb84...cbfdf0b
regressed commit: a2aba05

bisected with cargo-bisect-rustc v0.6.9

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc 

Trying to bisect the manual expansion code from #138596 (comment) in the 2024 edition doesn't seem find anything (it keeps running well past the introduction of the 2024 edition).

@Noratrieb Noratrieb added A-edition-2024 Area: The 2024 edition S-has-bisection Status: a bisection has been found for this issue T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Mar 17, 2025
@Noratrieb
Copy link
Member

Looks like this change: https://doc.rust-lang.org/edition-guide/rust-2024/temporary-tail-expr-scope.html#temporary-scope-may-be-narrowed

the block no longer extends the lifetime.

@Noratrieb
Copy link
Member

@danielhenrymantilla since the idea of the pin macro comes from you I think

@Noratrieb Noratrieb added A-pin Area: Pin regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels Mar 17, 2025
@yotamofek yotamofek marked this as a duplicate of #138600 Mar 17, 2025
@ehuss
Copy link
Contributor

ehuss commented Mar 17, 2025

I think I have a fix for this. Can anyone offhand think if there would be any problems with the following?

diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs
index 7fcd19f67ee..08c0f663588 100644
--- a/library/core/src/pin.rs
+++ b/library/core/src/pin.rs
@@ -2014,5 +2014,5 @@ unsafe impl<T: ?Sized> PinCoerceUnsized for *mut T {}
     //
     // See https://doc.rust-lang.org/1.58.1/reference/destructors.html#temporary-lifetime-extension
     // for more info.
-    $crate::pin::Pin::<&mut _> { __pointer: &mut { $value } }
+    $crate::pin::Pin::<&mut _> { __pointer: &mut ( $value ) }
 }

I don't see a particular reason that $value needs to be in a block. It must be an expression, and pre-2024 I don't see {} being different from () in terms of temporary extension (and temporary extension is the whole point here).

@ehuss
Copy link
Contributor

ehuss commented Mar 17, 2025

Oh, hm, maybe that won't work... There are some limitations with mutability....

@Nemo157
Copy link
Member

Nemo157 commented Mar 17, 2025

The main reason for the block is to force moving the value to take ownership, otherwise this works and is UB (playground):

#![feature(unsafe_pin_internals)]

struct Foo(std::marker::PhantomPinned);

fn main() {
    let mut value = Foo(std::marker::PhantomPinned);
    let indirect = &mut value;

    // let pinned = pin!(*indirect);
    let pinned = std::pin::Pin { __pointer: &mut (*indirect) };
    // let pinned = std::pin::Pin { __pointer: &mut {*indirect} };
    
    println!("{:p}", pinned.as_ref());
    
    let moved = value;
    println!("{:p}", &moved);
}

@ehuss
Copy link
Contributor

ehuss commented Mar 17, 2025

Hm, yea I understand now.

Crud, I'm not sure how to resolve that.

@lcnr
Copy link
Contributor

lcnr commented Mar 17, 2025

manually keep the pin! macro in a previous edition 🤔 ✨ idk how to do that, but at worst we can slap some new builtin #[rustc_X] macro on it :3

danielhenrymantilla added a commit to danielhenrymantilla/rust that referenced this issue Mar 17, 2025
danielhenrymantilla added a commit to danielhenrymantilla/rust that referenced this issue Mar 17, 2025
@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Mar 17, 2025

@ehuss' idea was on the right track, the semantics of braces are the one thing having changed; but @Nemo157's remark about parens themselves not forcing a value expression is indeed on point; this leaves us with the "typical" identity() equivalent of {}, or the less typical (..., ).0.

I've gone for the former in:


UPDATE: using identity breaks other forms of lifespan-of-temps extension, and my other convoluted workarounds could not overcome the limitations of the language under the 2024 edition.

I have listed the necessary changes in the language-or-compiler for us to feature a proper solution or workaround

@m-ou-se
Copy link
Member

m-ou-se commented Mar 19, 2025

@jdonszelmann made a temporary fix: #138717

This PR adds a #[rustc_macro_edition_2021] and applies it to pub macro pin.

If we land this now, we have some more time to think about how to express this properly in Rust 2024.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 21, 2025
…pkin

Add an attribute that makes the spans from a macro edition 2021, and fix pin on edition 2024 with it

Fixes a regression, see issue below. This is a temporary fix, super let is the real solution.

Closes rust-lang#138596
@bors bors closed this as completed in 7c2475e Mar 21, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 21, 2025
Rollup merge of rust-lang#138717 - jdonszelmann:pin-macro, r=WaffleLapkin

Add an attribute that makes the spans from a macro edition 2021, and fix pin on edition 2024 with it

Fixes a regression, see issue below. This is a temporary fix, super let is the real solution.

Closes rust-lang#138596
@m-ou-se
Copy link
Member

m-ou-se commented Mar 22, 2025

This is now fixed using a temporary solution that marks the expansion of pin!() as Rust 2021.

The expressivity problem in Rust 2024 is tracked in #138718

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition A-pin Area: Pin C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. S-has-bisection Status: a bisection has been found for this issue T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
9 participants