Stream: rustdoc

Topic: async functions show elided lifetimes #63037


view this post on Zulip Joshua Nelson (Dec 22 2020 at 17:50):

so I'm working on https://github.com/rust-lang/rust/issues/63037 and I found that the difference between the two is that the signature is different before rustdoc ever processes it

DEBUG rustdoc::clean clean_fn_sig: generics=Generics { params: [GenericParamDef { name: "\'_", kind: Lifetime }], where_predicates: [] }, decl=FnDecl { inputs: Arguments { values: [Argument { type_: BorrowedRef { lifetime: Some(Lifetime("\'_")), mutability: Not, type_: Primitive(Str) }, name: "foo" }] }, output: Return(ImplTrait([TraitBound(PolyTrait { trait_: ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Future", args: AngleBracketed { args: [], bindings: [TypeBinding { name: "Output", kind: Equality { ty: ResolvedPath { path: Path { global: false, res: Def(Struct, DefId(5:5688 ~ alloc[4799]::string::String)), segments: [PathSegment { name: "String", args: AngleBracketed { args: [], bindings: [] } }] }, param_names: None, did: DefId(5:5688 ~ alloc[4799]::string::String), is_generic: false } } }] } }] }, param_names: None, did: DefId(2:9354 ~ core[f78c]::future::future::Future), is_generic: false }, generic_params: [] }, None)])), c_variadic: false, attrs: Attributes { doc_strings: [], other_attrs: [], cfg: None, span: None, links: [], inner_docs: false } }
DEBUG rustdoc::clean clean_fn_sig: generics=Generics { params: [], where_predicates: [] }, decl=FnDecl { inputs: Arguments { values: [Argument { type_: BorrowedRef { lifetime: None, mutability: Not, type_: Primitive(Str) }, name: "foo" }] }, output: Return(ResolvedPath { path: Path { global: false, res: Def(Struct, DefId(5:5688 ~ alloc[4799]::string::String)), segments: [PathSegment { name: "String", args: AngleBracketed { args: [], bindings: [] } }] }, param_names: None, did: DefId(5:5688 ~ alloc[4799]::string::String), is_generic: false }), c_variadic: false, attrs: Attributes { doc_strings: [], other_attrs: [], cfg: None, span: None, links: [], inner_docs: false } }

view this post on Zulip Joshua Nelson (Dec 22 2020 at 17:50):

which is slightly concerning

view this post on Zulip Joshua Nelson (Dec 22 2020 at 17:51):

@tmandry do you know where in rustc async fn gets desugared?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 17:57):

yeah, this is literally all the preprocessing that happens: https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/src/librustdoc/visit_ast.rs#L311

view this post on Zulip Joshua Nelson (Dec 22 2020 at 17:58):

compiler/rustc_ast_lowering looks right

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:02):

aha! https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast_lowering/enum.AnonymousLifetimeMode.html

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:10):

ok, so I found the function adding the lifetimes: https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/compiler/rustc_ast_lowering/src/item.rs#L297-L310

before:

DEBUG rustc_ast_lowering::item lower_fn_kind(generics=Generics { params: [], where_clause: WhereClause { has_where_token: false, predicates: [], span: /home/joshua/rustc/src/librustdoc/async.rs:1:47: 1:47 (#0) }, span: /home/joshua/rustc/src/librustdoc/async.rs:1:26: 1:26 (#0) })

after:

DEBUG rustc_ast_lowering::item lower_fn_kind: generics=Generics { params: [GenericParam { hir_id: HirId { owner: DefId(0:3), local_id: 72 }, name: Fresh(0), attrs: [], bounds: [], span: /home/joshua/rustc/src/librustdoc/async.rs:1:32: 1:33 (#0), pure_wrt_drop: false, kind: Lifetime { kind: Elided } }], where_clause: WhereClause { predicates: [], span: /home/joshua/rustc/src/librustdoc/async.rs:1:47: 1:47 (#0) }, span: /home/joshua/rustc/src/librustdoc/async.rs:1:26: 1:26 (#0) }

and for comparison, the non-async version still has no params:

DEBUG rustc_ast_lowering::item lower_fn_kind(generics=Generics { params: [], where_clause: WhereClause { has_where_token: false, predicates: [], span: /home/joshua/rustc/src/librustdoc/async.rs:5:45: 5:45 (#0) }, span: /home/joshua/rustc/src/librustdoc/async.rs:5:24: 5:24 (#0) })
DEBUG rustc_ast_lowering::item lower_fn_kind: generics=Generics { params: [], where_clause: WhereClause { predicates: [], span: /home/joshua/rustc/src/librustdoc/async.rs:5:45: 5:45 (#0) }, span: /home/joshua/rustc/src/librustdoc/async.rs:5:24: 5:24 (#0) }

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:14):

and in particular the new params are from 'add_in_band_defs':

DEBUG rustc_ast_lowering add_in_band_defs: params=[], in_band_defs=[GenericParam { hir_id: HirId { owner: DefId(0:3), local_id: 72 }, name: Fresh(0), attrs: [], bounds: [], span: /home/joshua/rustc/src/librustdoc/async.rs:1:32: 1:33 (#0), pure_wrt_drop: false, kind: Lifetime { kind: Elided } }]

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:16):

so now I have a useful question to ask: @tmandry do you know why lower_generics_mut behaves differently for async fn and fn? https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/compiler/rustc_ast_lowering/src/lib.rs#L884-L893

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:20):

the really weird thing is that lower_generic_param never gets called

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:20):

maybe async fn adds an implicit where clause somehow?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:26):

aha! I was on the wrong track, these are actually added by lifetime_to_generic_param

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:28):

oh interesting, for fn, lifetime_to_generic_param isn't called at all

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:32):

so I guess somewhere a lifetime is getting marked as Plain("_") instead of Fresh? https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/compiler/rustc_ast_lowering/src/lib.rs#L779-L783 nope - the debugging says it's Fresh: https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/async.20functions.20show.20elided.20lifetimes/near/220718495

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:49):

ok so these are added to lifetimes_to_define somewhere:
DEBUG rustc_ast_lowering collect_in_band_defs: lifetimes_to_define=[(/home/joshua/rustc/src/librustdoc/async.rs:1:32: 1:33 (#0), Fresh(0))]

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:53):

somewhere

https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/compiler/rustc_ast_lowering/src/lib.rs#L839

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:57):

ok I finally found the issue - elided_ref_lifetime is called with AnonymousLifetimeMode::CreateParameter for async fn, but with PassThrough for `fn
https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/compiler/rustc_ast_lowering/src/lib.rs#L2577

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:57):

and so now my question is why this is different from a normal function:

            // Intercept when we are in an impl header or async fn and introduce an in-band
            // lifetime.
            // Hence `impl Foo for &u32` becomes `impl<'f> Foo for &'f u32` for some fresh
            // `'f`.

view this post on Zulip Joshua Nelson (Dec 22 2020 at 18:59):

https://github.com/rust-lang/rust/blob/0fe1dc6ac214d443369134177940b4d0111e1df6/compiler/rustc_ast_lowering/src/lib.rs#L1751-L1754

            // In `async fn`, argument-position elided lifetimes
            // must be transformed into fresh generic parameters so that
            // they can be applied to the opaque `impl Trait` return type.
            AnonymousLifetimeMode::CreateParameter

view this post on Zulip Joshua Nelson (Dec 22 2020 at 19:01):

added in https://github.com/rust-lang/rust/commit/749349fc9f7b12f212bca9ba2297e463328cb701

view this post on Zulip Joshua Nelson (Dec 22 2020 at 19:04):

and indeed changing that fixes rustdoc's issue:
image.png

view this post on Zulip Joshua Nelson (Dec 22 2020 at 19:04):

so @tmandry my question now is 'what will break' :laughing: is there a way to do this without creating fresh generic parameters? or does rustdoc need to change on its end?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 19:08):

oh I'm really sorry I've been pinging the wrong person

view this post on Zulip Joshua Nelson (Dec 22 2020 at 19:08):

@Taylor Cramer added this in https://github.com/rust-lang/rust/commit/749349fc9f7b12f212bca9ba2297e463328cb701

view this post on Zulip Joshua Nelson (Dec 22 2020 at 19:10):

here's the error when I naively remove it:

---- [ui] ui/suggestions/issue-72766.rs stdout ----
diff of stderr:

+   error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
+     --> $DIR/issue-72766.rs:7:48
+      |
+   LL |       pub async fn call(&self) -> Result<(), ()> {
+      |  _______________________-----____________________^
+      | |                       |
+      | |                       this data with an anonymous lifetime `'_`...
+   LL | |         Ok(())
+   LL | |     }
+      | |_____^ ...is captured here...
+      |
+   note: ...and is required to live as long as `'static` here
+     --> $DIR/issue-72766.rs:7:33
+      |
+   LL |     pub async fn call(&self) -> Result<(), ()> {
+      |                                 ^^^^^^^^^^^^^^
+

view this post on Zulip tmandry (Dec 22 2020 at 22:48):

my first thought is to find a way to add information to the new parameter about it being “synthetic” that rustdoc can use to ignore it

view this post on Zulip tmandry (Dec 22 2020 at 22:50):

or find some other way to detect the situation (lifetime parameters with the ‘_ name in a function signature aren’t something I’ve seen in real code before but maybe you can write them?)

view this post on Zulip Joshua Nelson (Dec 22 2020 at 22:51):

yes, you can write fn f(&'_ self)

view this post on Zulip Joshua Nelson (Dec 22 2020 at 22:52):

and I would prefer to keep that the same as the source code if possible

view this post on Zulip Joshua Nelson (Dec 22 2020 at 22:52):

otherwise things get way more complicated because rustdoc needs to know where the lifetime comes from, because Box<dyn Trait + '_> is different from Box<dyn Trait>

view this post on Zulip tmandry (Dec 22 2020 at 22:54):

I meant in the generics list, like: fn<‘_>(...)

view this post on Zulip tmandry (Dec 22 2020 at 22:54):

but I’m not sure that’s represented any differently in HIR

view this post on Zulip tmandry (Dec 22 2020 at 22:56):

anyway, it’s sounding like you want some way of representing synthetic lifetimes

view this post on Zulip Joshua Nelson (Dec 22 2020 at 22:57):

that would work, I think

view this post on Zulip Joshua Nelson (Dec 22 2020 at 22:57):

I'm not sure why the lifetimes are necessary in the first place

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:00):

I guess otherwise the return type is impl Future instead of impl Future + '_?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:00):

which defaults to 'static?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:00):

if there were some way to only add the lifetime to the return type that would be ideal

view this post on Zulip tmandry (Dec 22 2020 at 23:09):

well you need that but I think that’s already done in the desugaring

view this post on Zulip tmandry (Dec 22 2020 at 23:09):

this is so we can construct a synthetic type for the future / state machine returned by the function

view this post on Zulip tmandry (Dec 22 2020 at 23:10):

and we need to be able to name lifetimes in that type which correspond to the (elided) lifetimes in the function signature

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:12):

hmm, ok

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:12):

let me try ignoring anonymous lifetimes in function parameters like you suggested

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:12):

and the bogus fn f<'_, '_>

view this post on Zulip tmandry (Dec 22 2020 at 23:13):

so e.g. if you have async fn foo(x: &i32) we have to be able to create struct FooFuture<'a>(&'a i32), roughly

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:13):

ahhh that makes sense

view this post on Zulip tmandry (Dec 22 2020 at 23:13):

and resolve the opaque return type so it looks like fn foo<'a>(x: &'a i32) -> FooFuture<'a>

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:14):

so I guess my question is - why is this different from normal functions?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:14):

for fn foo(x: &i32) you still have an arbitrary lifetime, right?

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:14):

is it because you need to name it?

view this post on Zulip tmandry (Dec 22 2020 at 23:17):

yeah, you need to name the same lifetime in two different places

view this post on Zulip tmandry (Dec 22 2020 at 23:17):

for a single input/output lifetime elision can still work

view this post on Zulip tmandry (Dec 22 2020 at 23:18):

(I don't remember at what level elision is handled right now, I think desugaring?)

view this post on Zulip tmandry (Dec 22 2020 at 23:18):

but when there are multiple you need names

view this post on Zulip tmandry (Dec 22 2020 at 23:19):

so in general we need to handle cases where there are multiple unnamed lifetimes in the async fn signature

view this post on Zulip Joshua Nelson (Dec 22 2020 at 23:20):

oh that makes a lot of sense - I remember I tried fn f(a: &str, b: &str) with 1.34 (before this change was made) and it gave an error about multiple elided lifetimes

view this post on Zulip Joshua Nelson (Dec 23 2020 at 00:13):

tmandry said:

anyway, it’s sounding like you want some way of representing synthetic lifetimes

hmm, I thought I could tell this apart by looking at InBand vs Elided, but these lifetimes are marked as Elided for some reason :/

view this post on Zulip Joshua Nelson (Dec 23 2020 at 00:14):

what's the difference between 'elided' and 'inband' anyway?

view this post on Zulip Joshua Nelson (Dec 23 2020 at 00:16):

ok well good news, filtering out Elided lifetimes gets rid of the <'_> :)

view this post on Zulip Joshua Nelson (Dec 23 2020 at 00:16):

and that should always be safe because you can't write <'_> in user code

view this post on Zulip Joshua Nelson (Dec 23 2020 at 01:42):

https://github.com/rust-lang/rust/pull/80319

view this post on Zulip Joshua Nelson (Dec 23 2020 at 01:47):

@tmandry thanks for the help with this :)

view this post on Zulip tmandry (Dec 23 2020 at 01:48):

glad I could help!

view this post on Zulip tmandry (Dec 23 2020 at 02:12):

@Joshua Nelson left a review, btw

view this post on Zulip Joshua Nelson (Dec 23 2020 at 02:30):

left a response


Last updated: Oct 11 2021 at 22:34 UTC