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

Error with nested async fn and anonymous lifetimes #63225

Closed
udoprog opened this issue Aug 2, 2019 · 20 comments

Comments

@udoprog
Copy link
Contributor

commented Aug 2, 2019

The following breaks on nightly (see playground)

#![feature(async_await)]

struct Foo<'a>(&'a ());

impl Foo<'_> {
    fn test() {
        async fn test() {}
    }
}

With:

error[E0261]: use of undeclared lifetime name `'_`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0261`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

CC: dtolnay/async-trait#18

@Centril

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

Reduced:

#![feature(async_await)]

struct Foo<'a>(&'a ());

impl<'a> Foo<'a> {
    fn test() {
        async fn test() {}
    }
}
@udoprog

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

Forgot to mention in case this helps, giving the inner fn a matching lifetime avoids this issue for some reason:

#![feature(async_await)]

struct Foo<'a>(&'a ());

impl<'a> Foo<'a> {
    fn test() {
        async fn test<'a>() {}
    }
}
@cramertj

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

The reason this is happening is that the desugared version of async fn applies all in-scope lifetimes to the returned existential type, which in this case is creating -> Fut<'_>, which is invalid. I'm not sure exactly what the best fix is for this, though, since elided lifetimes in the impl signature are intentionally unnameable. The closest idea I have would be to create some special new lifetime that is '_ but can refer to lifetimes originating from the impl header. cc @nikomatsakis who may have opinions.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

Hmm @cramertj wouldn't you expect the nested async fn to not inherit anything from its enclosing scopes? Seems like a bug to me that it does? Am I confused about something?

@cramertj

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

@nikomatsakis The function in the middle is actually a bit of a source of confusion here, although it does indicate that there's a second bug since we shouldn't be inheriting generics since the nested async fn isn't part of the impl.

The bug I was focusing on is better illustrated by this example:

#![feature(async_await)]

struct Foo<'a>(&'a u8);

impl Foo<'_> {
    async fn bar() {}
}

which produces this error:

error[E0261]: use of undeclared lifetime name `'_`

error: aborting due to previous error

This is because the code is transformed into

impl Foo<'_> {
    fn bar() -> OpaqueTy<'_> {}
}

and in that case '_ does not refer back to the usage from the impl header.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

@cramertj I see. Indeed, I would expect that, internally, we'd have some way to refer back to the '_ from the impl header. I believe this should be possible, no? In particular, the '_ in impls expands (internally) to LifetimeName::Param(ParamName::Fresh(N)) for some integer N, which I believe is basically treated like any other named lifetime.

@cramertj

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

@nikomatsakis Yep, I would've expected so as well-- I'm not sure why that isn't working, I'd have to take a look.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

@cramertj I was looking and I think the bug lies here:

let lifetime_params: Vec<(Span, ParamName)> =
this.in_scope_lifetimes
.iter().cloned()
.map(|ident| (ident.span, ParamName::Plain(ident)))
.chain(this.lifetimes_to_define.iter().cloned())
.collect();

Probably in_scope_lifetimes needs to be a Vec<ParamName> instead, or we need to use something else for this purpose.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

A closer reading suggests I was mistaken. The '_ lifetimes from the impl should show up in lifetimes_to_define -- in_scope_lifetimes includes only the explicitly declared lifetimes written by the user on the impl. I'm not quite sure what the problem is yet.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

OK, no, I was correct. When processing impl items, we convert the '_ lifetimes into in_scope_lifetime entries.

EDIT: Er, I'm still not entirely sure. Reading more. =)

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

OK, I have a fix for the async fn method issue, but not the nested async fn item issue.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

I forked off #63500 for the method issue

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

@cramertj I didn't prep a fix for this one yet. It seems obvious that some internal state is not being reset when we enter a new item.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

but if you have a chance to investigate, would be appreciated

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Presumably the problem is that visit_item doesn't "reset" the in_scope_lifetimes vector -- it only seems to extend it...

self.lctx.with_parent_item_lifetime_defs(hir_id, |this| {

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Indeed, in-band lifetimes are broken in the same way. This fails to compile (playground):

#![feature(in_band_lifetimes)]

struct Foo<'a> {
    x: &'a u32
    
}

impl Foo<'a> {
    fn method(&self) {
        fn blah(f: Foo<'a>) { }
    }
}
@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Working on a possible fix

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

Yep, have a fix.

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Inband lifetimes looks similar to #52532 fwiw

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

@nikomatsakis nikomatsakis self-assigned this Aug 13, 2019

Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019

Rollup merge of rust-lang#63501 - nikomatsakis:issue-63500-async-anon…
…-impl-lifetime, r=cramertj

use `ParamName` to track in-scope lifetimes instead of Ident

Also, clear in-scope lifetimes when visiting nested items.

Fixes rust-lang#63500.
Fixes rust-lang#63225.
Fixes rust-lang#52532.

r? @cramertj

Centril added a commit to Centril/rust that referenced this issue Aug 14, 2019

Rollup merge of rust-lang#63501 - nikomatsakis:issue-63500-async-anon…
…-impl-lifetime, r=cramertj

use `ParamName` to track in-scope lifetimes instead of Ident

Also, clear in-scope lifetimes when visiting nested items.

Fixes rust-lang#63500.
Fixes rust-lang#63225.
Fixes rust-lang#52532.

r? @cramertj

@bors bors closed this in #63501 Aug 14, 2019

Centril added a commit to Centril/rust that referenced this issue Aug 20, 2019

Rollup merge of rust-lang#63209 - Centril:stabilize-async-await, r=cr…
…amertj

Stabilize `async_await` in Rust 1.39.0

Here we stabilize:
- free and inherent `async fn`s,
- the `<expr>.await` expression form,
- and the `async move? { ... }` block form.

Closes rust-lang#62149.
Closes rust-lang#50547.

All the blockers are now closed.

<details>
- [x] FCP in rust-lang#62149
- [x] rust-lang#61949; PR in rust-lang#62849.
- [x] rust-lang#62517; PR in rust-lang#63376.
- [x] rust-lang#63225; PR in rust-lang#63501
- [x] rust-lang#63388; PR in rust-lang#63499
- [x] rust-lang#63500; PR in rust-lang#63501
- [x] rust-lang#62121 (comment)
    - [x] Some tests for control flow (PR rust-lang#63387):
          - `?`
          - `return` in `async` blocks
          - `break`
    - [x] rust-lang#61775 (comment), i.e. tests for rust-lang#60944 with `async fn`s instead). PR in rust-lang#63383

</details>

r? @cramertj
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.