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

2024 No migration for a particular RPIT scenario (async?) #132819

Closed
ehuss opened this issue Nov 9, 2024 · 3 comments · Fixed by #132817
Closed

2024 No migration for a particular RPIT scenario (async?) #132819

ehuss opened this issue Nov 9, 2024 · 3 comments · Fixed by #132817
Labels
A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Nov 9, 2024

Sorry, I'm not certain how to describe this scenario or why it doesn't seem to work. This could also probably be minimized more.

I tried this code:

#![warn(rust_2024_compatibility)]
use std::pin::Pin;

pub struct AttachContainerResults {
    pub input: Pin<Box<dyn AsyncWrite + Send>>,
}

pub trait AsyncWrite {}
struct S;
impl AsyncWrite for S {}

pub async fn attach_container(this: &i32) -> AttachContainerResults {
    let write = process_upgraded(this).await;

    AttachContainerResults {
        input: Box::pin(write),
    }
}

async fn process_upgraded(_this: &i32) -> impl AsyncWrite {
    S
}

I expected to see this happen: Should generate a warning about it not working in 2024. At least, I cannot correlate the error with the scenarios described in the migration docs that describe scenarios that are not supported.

Instead, this happened: This does not generate any warnings about it not working in 2024. After migrating to 2024, you get an error:

error: lifetime may not live long enough
  --> src/main.rs:16:16
   |
12 | pub async fn attach_container(this: &i32) -> AttachContainerResults {
   |                                     - let's call the lifetime of this reference `'1`
...
16 |         input: Box::pin(write),
   |                ^^^^^^^^^^^^^^^ coercion requires that `'1` must outlive `'static`

I believe the solution here is to add:

     }
 }
 
-async fn process_upgraded(_this: &i32) -> impl AsyncWrite {
+async fn process_upgraded(_this: &i32) -> impl AsyncWrite + use<> {
     S
 }

From messing around, it seems like the async part of process_upgraded is the key factor that is preventing the impl_trait_overcaptures lint from firing.

Meta

rustc --version --verbose:

rustc 1.84.0-nightly (b91a3a056 2024-11-07)
binary: rustc
commit-hash: b91a3a05609a46f73d23e0995ae7ebb4a4f429a5
commit-date: 2024-11-07
host: aarch64-apple-darwin
release: 1.84.0-nightly
LLVM version: 19.1.3
@ehuss ehuss added A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. labels Nov 9, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 9, 2024
@compiler-errors
Copy link
Member

Isn't this just a duplicate of #132809 (comment) ? This should be fixed by #132817.

@compiler-errors compiler-errors linked a pull request Nov 9, 2024 that will close this issue
@ehuss
Copy link
Contributor Author

ehuss commented Nov 9, 2024

Oh, indeed it probably is! I didn't make the connection since the errors were different.

@compiler-errors
Copy link
Member

compiler-errors commented Nov 9, 2024

Overcapturing will result in a wide array of different borrowck errors that all have to do with things being required to live longer. Only some of them are intercepted by #131186, but due to the nature of NLL, we won't be able to detect all of them.

@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 9, 2024
@jieyouxu jieyouxu added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 10, 2024
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 C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants