rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.77k stars 12.5k forks source link

RPITIT "captures lifetime that does not appear in bounds" #128752

Open tmandry opened 1 month ago

tmandry commented 1 month ago

I tried this code: playground

trait MyTrait {
    fn foo<'a, 'b>(&'a self, x: &'b i32) -> impl Future<Output = i32>;
}

trait ErasedMyTrait {
    fn foo<'life0, 'life1, 'dynosaur>(&'life0 self, x: &'life1 i32)
    -> Pin<Box<dyn Future<Output = i32> + 'dynosaur>>
    where
    'life0: 'dynosaur,
    'life1: 'dynosaur;
}

struct DynMyTrait<T: ErasedMyTrait> {
    ptr: T,
}

impl<T: ErasedMyTrait> MyTrait for DynMyTrait<T> {
    fn foo<'a, 'b>(&'a self, x: &'b i32) -> impl Future<Output = i32> {
        self.ptr.foo(x)
    }
}

Diagnostics

I got a nonsensical error message:

error[E0700]: hidden type for `impl Future<Output = i32>` captures lifetime that does not appear in bounds
  --> src/lib.rs:22:9
   |
21 |     fn foo<'a, 'b>(&'a self, x: &'b i32) -> impl Future<Output = i32> {
   |                --                           ------------------------- opaque type defined here
   |                |
   |                hidden type `Pin<Box<(dyn Future<Output = i32> + 'b)>>` captures the lifetime `'b` as defined here
22 |         self.ptr.foo(x)
   |         ^^^^^^^^^^^^^^^
   |
help: to declare that `impl Future<Output = i32>` captures `'b`, you can add an explicit `'b` lifetime bound
   |
21 |     fn foo<'a, 'b>(&'a self, x: &'b i32) -> impl Future<Output = i32> + 'b {
   |                                                                       ++++

This is an RPITIT so the lifetime should not have to appear in the bounds to be captured. As you might expect, adding the + 'b leads to a different error, and using the Captures trick does not work either (in fact, it is quite a mess of surprising errors: playground).

Should this compile?

Diagnostics aside, it's understandable that this wouldn't compile, because the compiler doesn't know what to do with the extra 'dynosaur lifetime in ErasedMyTrait::foo. It would have to generate an intersection lifetime to get the full return type of that method (the 'dynosaur in Pin<Box<dyn Future<Output = i32> + 'dynosaur>>.

Adding a lifetime that corresponds to 'dynosaur to the original trait does the trick:

trait MyTrait {
    fn foo<'a, 'b, 'd>(&'a self, x: &'b i32) -> impl Future<Output = i32>
    where 'a: 'd, 'b: 'd;
}

As I alluded to above, I don't think have we to know which lifetime 'dynosaur corresponds to to know that the original program is sound. Representing it as the intersection of 'a and 'b should be enough to see that our Pin<Box<dyn Future>> satisfies -> impl Future in the signature of the method we're implementing.

Through some magic it isn't necessary if we use async fn directly; this impl compiles fine:

impl<T: ErasedMyTrait> MyTrait for DynMyTrait<T> {
    async fn foo<'a, 'b>(&'a self, x: &'b i32) -> i32 {
        self.ptr.foo(x).await
    }
}

I'm pretty much expecting to hear that this is too hard to support now, but the async fn chicanery gives me hope that it can be supported sooner.

Meta

rustc version:

1.82.0-nightly
(2024-08-05 e57f3090aec33cdbf660)
tmandry commented 1 month ago

cc @spastorino @compiler-errors

compiler-errors commented 1 month ago

Yeah, you're right that this is not possible to fix. Opaques can't capture intersection lifetimes, since they're local to the body of the function and not represented by any of the params on the function that defines the RPITIT -- this is what the lifetime 'dynosaur is in this example is, on the caller side.

Sorry that the diagnostic does not make this clear this is the problem -- it shouldn't be saying "captures the lifetime 'b as defined here" because if we were trying to capture the lifetime 'b, then we wouldn't have a problem here. 'b is indeed captured by opaque by default since RPITITs capture all in scope lifetimes.

The fact that the async example works is because that async {} block (coming out of the async desugaring) doesn't actually have anything to do with object type. All it does is capture upvars (namely, &'a self and x: &'b i32, which are OK because 'a and 'b are captured by the RPITIT) -- the object type that we end up awaiting is interior to the async block, so we don't care about it for the purposes of outlives.

I'll look into making the diagnostic less misleading, though.

compiler-errors commented 1 month ago

128753 suppresses the part that says "captures the lifetime 'b" and "add + 'b", since that's just inaccurate. After that PR, it says "captures '_", which is vague, but at least it's not literally lying about what region is being captured.

tmandry commented 1 month ago

Yeah, you're right that this is not possible to fix. Opaques can't capture intersection lifetimes, since they're local to the body of the function and not represented by any of the params on the function that defines the RPITIT

That's an implementation choice though, isn't it? At least in my conceptual framework, an intersection lifetime is purely a function of those input lifetimes. In other words, this refinement should be possible.

trait MyTrait {
    fn foo<'a, 'b>(&'a self, x: &'b i32) -> impl Future<Output = i32>;
}

impl MyTrait for Foo {
    fn foo<'a, 'b>(&'a self, x: &'b i32) -> Pin<Box<dyn Future<Output = i32> + intersection<'a, 'b>>> { todo!() }
}

Happy to be told either that my conceptual framework is wrong or that it is theoretically sound but we aren't ready to represent this anytime soon.. just trying to understand what is meant by "impossible" here :)

The fact that the async example works is because that async {} block (coming out of the async desugaring) doesn't actually have anything to do with object type. All it does is capture upvars (namely, &'a self and x: &'b i32, which are OK because 'a and 'b are captured by the RPITIT) -- the object type that we end up awaiting is interior to the async block, so we don't care about it for the purposes of outlives.

Thanks for that explanation. I bet we can emulate this in the proc macro @spastorino and I have been working on.

I'll look into making the diagnostic less misleading, though.

Thank you!

compiler-errors commented 1 month ago

At least in my conceptual framework, an intersection lifetime is purely a function of those input lifetimes. In other words, this refinement should be possible.

Well, the real issue here is that the opaque type inference algorithm relies on being able to map a hidden type of an opaque back onto the lifetime parameters that the opaque captures. When we have some:

type RPIT<'a, 'b> = Pin<Box<dyn Future<Output = i32> + '1>>
//        ^^^^^^ These are the captures of the opaque

...and all we know from the environment is that 'a: '1 and 'b: '1, that does not give us sufficient information to actually map '1 to one of those regions (since 'a and 'b are totally unrelated regions), which we must for opaque types to work in Rust.

As an aside, you can observe that this is fixed if we added 'a: 'b or 'b: 'a to our method signature (even though I totally agree that's an arbitrary restriction to have), since saying one region outlives the other gives us an obvious choice of a least-upper-bound.

I don't think calling this an implementation choice is particularly useful here, since it overlooks the things that makes it hard in the implementation. I could call anything that Rust does an implementation detail, but the fact here is that Rust does not have first-class intersection regions in its type system (like, notice that you have to go out of your way to invent a new syntax to represent a region to put into the dyn Future).

I bet we can emulate this in the proc macro spastorino and I have been working on.

I don't believe this can be emulated in general, for the same reasons that futures cannot always be lowered into state machines because they can feature existential inner regions that can't be named. Specifically, if you wrote some:

enum ManuallyDesugaredStateMachine<'a, 'b> {
    State1 {
        self_: &'a Foo,
        x: &'b i32,
    },
    State2 {
        fut: Pin<Box<dyn Future<Output = i32> + /* what do we put here? */>>,
    },
    Done,
}

Then you'd either need to introduce some interesection region directly to the enum like 'd where 'a: 'd, 'b: 'd which brings us exactly back to where we started, or we would need an anonymous existential lifetime which does not exist right now in the type system.

tmandry commented 1 month ago

I could call anything that Rust does an implementation detail, but the fact here is that Rust does not have first-class intersection regions in its type system (like, notice that you have to go out of your way to invent a new syntax to represent a region to put into the dyn Future).

Yeah this matches my understanding. I don't think "implementation detail" was a good choice of words. What I meant was that there's nothing fundamentally unsound about this pattern and a language like Rust could support it. But I get that I am totally glossing over its interaction with other features in the type system.

I don't believe this can be emulated in general, for the same reasons that futures cannot always be lowered into state machines because they can feature existential inner regions that can't be named.

I meant with unsafe code. In this case I would stick a 'static there, restrict visibility, and list all input lifetimes as parameters of the owning struct/enum to prevent use after free. I think this works? https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7f276c43d93fdc257576c8dd60c0ab06

compiler-errors commented 1 month ago

'static may work, but you may get mysterious borrowck errors.

compiler-errors commented 1 month ago

What I meant was that there's nothing fundamentally unsound about this pattern.

I don't believe that there's anything fundamentally unsound about this.