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

Explicit lifetime required on scoped value #123195

Closed
aumetra opened this issue Mar 29, 2024 · 7 comments
Closed

Explicit lifetime required on scoped value #123195

aumetra opened this issue Mar 29, 2024 · 7 comments
Labels
A-lifetimes Area: Lifetimes / regions C-discussion Category: Discussion or questions that doesn't represent real issues.

Comments

@aumetra
Copy link

aumetra commented Mar 29, 2024

This is an issue with the scoped-futures library, which is a Future-specific implementation of a technique outlined in this article.

On stable code I wrote compiles just fine, but on beta it fails. In simple cases this is solved by adding a lifetime, but in other cases the fix can get rather gnarly.

I managed to minimize it down into a small sample:

Code

I tried this code:

use std::marker::PhantomData;

pub struct ScopedWrapper<'upper_bound, 'subject> {
    scope: PhantomData<&'subject &'upper_bound ()>,
}

pub fn works_on_stable<'a, F>(reference: &(), scoped: F)
where
    for<'b> F: Fn(&'b ()) -> ScopedWrapper<'b, 'a>,
{
    scoped(reference);
}

I expected to see this happen: It simply compiles.

Instead, this happened: A lifetime-related compiler error happened.

Error message:

aumetra@gazenot:~/regression-min-repr$ cargo +beta build
   Compiling regression-min-repr v0.1.0 (/home/aumetra/regression-min-repr)
error[E0621]: explicit lifetime required in the type of `reference`
  --> src/main.rs:15:5
   |
11 | pub fn works_on_stable<'a, F, Fut>(reference: &(), scoped: F)
   |                                               --- help: add explicit lifetime `'a` to the type of `reference`: `&'a ()`
...
15 |     scoped(reference);
   |     ^^^^^^^^^^^^^^^^^ lifetime `'a` required

For more information about this error, try `rustc --explain E0621`.
error: could not compile `regression-min-repr` (bin "regression-min-repr") due to 1 previous error

Version it worked on

rustc 1.77.0 (aedd173a2 2024-03-17)
binary: rustc
commit-hash: aedd173a2c086e558c2b66d3743b344f977621a7
commit-date: 2024-03-17
host: x86_64-unknown-linux-gnu
release: 1.77.0
LLVM version: 17.0.6

Version with regression

The latest beta

rustc --version --verbose:

rustc 1.78.0-beta.3 (4147533e0 2024-03-27)
binary: rustc
commit-hash: 4147533e05ee20c4fcc432736e7feeafa46521cd
commit-date: 2024-03-27
host: x86_64-unknown-linux-gnu
release: 1.78.0-beta.3
LLVM version: 18.1.2

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

@aumetra aumetra added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Mar 29, 2024
@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. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Mar 29, 2024
@compiler-errors
Copy link
Member

This is likely due to #118882. I don't believe this is a regression, since this is a correct error message -- the lifetime on reference: &() does not outlive 'a, which is required for ScopedWrapper to be valid.

@aumetra
Copy link
Author

aumetra commented Mar 29, 2024

Ah, makes sense. I thought there was some inference going on.

@aumetra
Copy link
Author

aumetra commented Mar 29, 2024

What about this? (I'm sure I'm just misreading/thinking wrong somehere here, too..)

use std::{
    future::Future,
    marker::PhantomData,
    ops::{Deref, DerefMut},
    pin::Pin,
};

pub trait ScopedFuture<'upper_bound, 'subject, Bound = ImpliedLifetimeBound<'upper_bound, 'subject>>:
    Future
where
    Bound: sealed::Sealed,
{
}

pub type ImpliedLifetimeBound<'upper_bound, 'subject> = PhantomData<&'subject &'upper_bound ()>;

impl<'upper_bound: 'subject, 'subject, Fut: Future + 'subject> ScopedFuture<'upper_bound, 'subject>
    for Fut
{
}

mod sealed {
    pub trait Sealed {}
    impl<'upper_bound, 'a> Sealed for super::ImpliedLifetimeBound<'upper_bound, 'a> {}
}

/// A [`Future`] wrapper type that imposes an upper bound on its lifetime's duration.
#[derive(Clone, Debug)]
pub struct ScopedFutureWrapper<'upper_bound, 'subject, Fut> {
    future: Fut,
    scope: ImpliedLifetimeBound<'upper_bound, 'subject>,
}

impl<'upper_bound, 'subject, Fut> ScopedFutureWrapper<'upper_bound, 'subject, Fut> {
    pin_utils::unsafe_pinned!(future: Fut);
}

impl<'upper_bound, 'subject, Fut: Future> Future
    for ScopedFutureWrapper<'upper_bound, 'subject, Fut>
{
    type Output = Fut::Output;
    fn poll(
        self: Pin<&mut Self>,
        cx: &mut core::task::Context<'_>,
    ) -> core::task::Poll<Self::Output> {
        self.future().poll(cx)
    }
}

struct Resource;

struct Wrapper<'a>(&'a mut Resource);

impl Deref for Wrapper<'_> {
    type Target = Resource;

    fn deref(&self) -> &Self::Target {
        self.0
    }
}

impl DerefMut for Wrapper<'_> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.0
    }
}

struct Owner {
    resource: Resource,
}

impl Owner {
    async fn obtain(&mut self) -> Wrapper<'_> {
        Wrapper(&mut self.resource)
    }
}

async fn works_on_stable<'a, F, Fut>(owner: &'a mut Owner, func: F)
where
    for<'conn> F: FnOnce(&'conn mut Resource) -> ScopedFutureWrapper<'conn, 'a, Fut>,
    Fut: Future,
{
    let mut conn = owner.obtain().await;
    func(&mut conn).await;
}

A bit more involved, yes. Here's the issue:

  1. Technically these bounds do not require the argument to have the bound 'a
  2. Theoretically the value has the bound 'a since Owner is mutably borrowed for the lifetime 'a and the values it produces should have the theoretical maximum lifetime of 'a (unless I'm misreading/misinterpreting lifetimes in my head here)

@compiler-errors
Copy link
Member

compiler-errors commented Mar 29, 2024

So the ScopedFutureWrapper requires 'conn at the callsite (func(&mut conn)) to outlive 'a. The DerefMut impl lets you get a &mut Resource from &mut Wrapper, but that's going to be upper-bounded by the local lifetime you take on conn: Wrapper<'local>. You can't take a local reference that lives for longer than 'a since that's a lifetime external to the function body.

async fn works_on_stable<'a, F, Fut>(owner: &'a mut Owner, func: F)
where
    for<'conn> F: FnOnce(&'conn mut Resource) -> ScopedFutureWrapper<'conn, 'a, Fut>,
    Fut: Future,
{
    let mut conn = owner.obtain().await;
-   func(&mut conn).await;
+   func(conn.0).await;
}

You can move the mutable reference out of conn to consume it, giving you a reference that actually lives for the full lifetime 'a. Not certain that's what you actually want, though.

@aumetra
Copy link
Author

aumetra commented Mar 29, 2024

It's interesting that the local lifetime is upper-bounded. It's not something I would have expected here, since it's all derived from the top and just borrows all the way down.
Especially since the execution is guaranteed to be finished by the time we return our borrow.

(also about moving the reference out of the structure, that's a bit hard in the real-world case I'm encountering it in. The real-world case is a database connection borrowed from a pool, and the connection is wrapped inside a wrapper to return it back on drop, etc etc.)

@compiler-errors
Copy link
Member

compiler-errors commented Mar 29, 2024

@aumetra -- that has to do with mutable reborrows and the signature of DerefMut. You can't borrow a mutable reference for longer than you have a lease on it, which is related to the borrow you take here:

  func(&mut conn).await;
       ^^^^ this borrow of a local variable, which can live no longer than the function body

You may want to rework the way you're using DerefMut to do something else.

@aumetra
Copy link
Author

aumetra commented Mar 29, 2024

Thing is, the execution of the function is guaranteed to finish while I have the lease, is it not?
I quite honestly do not get how this compiler error even exists. The execution of the function is guaranteed to finish before the lease expires.
In what way would this be unsound?

 ┌────────────────────────────────────┐ 
 │                                    │ 
 │          Outer function            │ 
 │                                    │ 
 │  ┌──────────────────────────────┐  │ 
 │  │                              │  │ 
 │  │  ┌──── Lease begins          │  │ 
 │  │  │                           │  │ 
 │  │  │   ┌────────────────────┐  │  │ 
 │  │  │   │                    │  │  │ 
 │  │  │   │ Function execution │  │  │ 
 │  │  │   │                    │  │  │ 
 │  │  │   └────────────────────┘  │  │ 
 │  │  │                           │  │ 
 │  │  └───► Lease expires         │  │ 
 │  │                              │  │ 
 │  └──────────────────────────────┘  │ 
 │                                    │ 
 └────────────────────────────────────┘ 

The expression of this idea is probably just wrong in terms of trait bounds.
Anyway, I feel like this is just going out of the scope of a bug report issue. I'll just go and do whatever

@aumetra aumetra closed this as completed Mar 29, 2024
@aumetra aumetra closed this as not planned Won't fix, can't repro, duplicate, stale Mar 29, 2024
@saethlin saethlin added A-lifetimes Area: Lifetimes / regions C-discussion Category: Discussion or questions that doesn't represent real issues. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. C-bug Category: This is a bug. 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 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions C-discussion Category: Discussion or questions that doesn't represent real issues.
Projects
None yet
Development

No branches or pull requests

4 participants