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

TAIT: hidden type captures lifetime that does not appear in bounds #104691

Closed
Dirbaio opened this issue Nov 21, 2022 · 10 comments
Closed

TAIT: hidden type captures lifetime that does not appear in bounds #104691

Dirbaio opened this issue Nov 21, 2022 · 10 comments
Assignees
Labels
F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]`

Comments

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 21, 2022

playground

pub trait Trait {
    type FooFn<'a>:  FnOnce() + 'a where Self: 'a;

    fn foo(&mut self) -> Self::FooFn<'_>;
}

struct Thingy(u32);

impl Trait for &Thingy {
    type FooFn<'a> = impl FnOnce() + 'a where Self: 'a;
    fn foo(&mut self) -> Self::FooFn<'_> {
        move || {
            println!("{}", self.0);
        }
    }
}

fails with

error[E0700]: hidden type for `<&Thingy as Trait>::FooFn<'_>` captures lifetime that does not appear in bounds
  --> src/lib.rs:16:9
   |
13 |   impl Trait for &Thingy {
   |                  - hidden type `[closure@src/lib.rs:16:9: 16:16]` captures the anonymous lifetime as defined here
...
16 | /         move || {
17 | |             println!("{}", self.0);
18 | |         }
   | |_________^

while this used to work before. I think because the where Self: 'a part takes care of the capture (?).

Not present in nightly yet, found it by building 1cbc459 from git master. Bisected it to 7fe6f36 which is #103491 @cjgillot

@rustbot label F-type_alias_impl_trait

@rustbot rustbot added the F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` label Nov 21, 2022
@cjgillot cjgillot self-assigned this Nov 21, 2022
@cjgillot
Copy link
Contributor

Is this a bug or a bugfix? If I name lifetimes:

pub trait Trait {
    type FooFn<'a>:  FnOnce() + 'a where Self: 'a;

    fn foo(&mut self) -> Self::FooFn<'_>;
}

struct Thingy;

impl<'b> Trait for &'b Thingy {
    type FooFn<'a> = impl FnOnce() + 'a where Self: 'a;
    fn foo<'c>(&'c mut self) -> Self::FooFn<'c> {
        move || {
            let _ = self;
        }
    }
}

The provided code does indeed capture the implicit lifetime 'b: the closure upvar is &'c mut 'b Thingy.
So, IMO, the error is legitimate, and previous implementation erroneously considered 'b as allowed, which is inconsistent with how RPIT worked. Do you agree with those TAIT semantics @oli-obk?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2022

I tried the same thing but realized the original also passes on playground until tomorrow. So I don't know if explicit lifetimes actually change anything?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2022

But yea, I'd expect this to fail without a 'b bound

@Dirbaio
Copy link
Contributor Author

Dirbaio commented Nov 22, 2022

Shouldn't it be taken care of by where Self: 'a? It means any possible lifetime in Self outlives 'a, so saying "FooFn captures 'a" should be "strong enough" (?)

Also, forcing the user to write every single lifetime is going to be quite unergonomic.

@Dirbaio
Copy link
Contributor Author

Dirbaio commented Nov 23, 2022

I've noticed let _ = self is not enough to trigger the error. It does trigger in the original code, but it doesn't when I extract it to a separate crate, nor in the playground. Not sure why. println!("{}", self.0); always triggers it. I've updated the first post.

nightly-2022-11-22 is out now, so it's failing in the playground now.

@Dirbaio
Copy link
Contributor Author

Dirbaio commented Nov 23, 2022

@oli-obk how would you fix the code so that it works? I've tried + 'a + 'b and variations of it, but can't get it to compile.

Also, I've found it compiles if you add extra indirection through a method without changing any signature. This really makes me believe the original code is sound and should be accepted. playground

impl Trait for &Thingy {
    type FooFn<'a> = impl FnOnce() + 'a where Self: 'a;
    fn foo(&mut self) -> Self::FooFn<'_> {
        self.foo_inner()
    }
}

impl Thingy {
    fn foo_inner(&self) -> impl FnOnce() + '_ {
        move || {
            println!("{}", self.0);
        }
    }
}

This issue is breaking a lot of uses of GAT+TAIT for manually-desugared async traits: here, here, here, here, here, here

@oli-obk
Copy link
Contributor

oli-obk commented Nov 23, 2022

impl Thingy {
    fn foo_inner(&self) -> impl FnOnce() + '_ {
        move || {
            println!("{}", self.0);
        }
    }
}

this avoids capturing the &mut &Thingy in foo, avoiding the problematic case you're encountering.

Shouldn't it be taken care of by where Self: 'a? It means any possible lifetime in Self outlives 'a, so saying "FooFn captures 'a" should be "strong enough" (?)

that's the wrong end of where we should be figuring out what's wrong here. I am now realizing that

pub trait Trait {
    type FooFn<'a>:  FnOnce() + 'a where Self: 'a;

    fn foo(&mut self) -> Self::FooFn<'_>;
}

struct Thingy;

impl<'b> Trait for &'b Thingy {
    type FooFn<'a> = impl FnOnce() + 'a where Self: 'a;
    fn foo<'c>(&'c mut self) -> Self::FooFn<'c> {
        move || {
            let _ = self;
        }
    }
}

while correctly causing

The provided code does indeed capture the implicit lifetime 'b: the closure upvar is &'c mut 'b Thingy.

Should also allow us to realize that &'c mut 'b Thingy means 'b outlives 'c and thus we may capture 'b if 'c is in the bounds.

But that doesn't even work for RPIT (playground):

struct Thingy(u32);

fn foo<'c, 'b>(x: &'c mut &'b Thingy) -> impl FnOnce() + 'c {
    move || {
        println!("{}", x.0);
    }
}

I'm moderately certain this will compile with -Zchalk and will be solved by implied bounds

@cjgillot
Copy link
Contributor

@oli-obk I think the presence of a mut in &'c mut 'b Thingy prevents leveraging the 'b outlives 'c bound. With shared reference, all works well, and the code is accepted. With a mut, we have no guarantee that 'b itself isn't relevant eg. for assignment. Or am I wrong in this?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 24, 2022

Ah. If we didn't correctly handle variance here, we'd be able to write something with 'c lifetime into something with 'b lifetime.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 24, 2022

The foo_inner situation works, because it changes that reference to &&Thingy with not mutable refs.

Closing as working as intended

@oli-obk oli-obk closed this as completed Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]`
Projects
None yet
Development

No branches or pull requests

4 participants