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

handle elision in async fn correctly #63499

Merged
181 changes: 56 additions & 125 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,49 +341,6 @@ enum AnonymousLifetimeMode {

/// Pass responsibility to `resolve_lifetime` code for all cases.
PassThrough,

/// Used in the return types of `async fn` where there exists
/// exactly one argument-position elided lifetime.
///
/// In `async fn`, we lower the arguments types using the `CreateParameter`
/// mode, meaning that non-`dyn` elided lifetimes are assigned a fresh name.
/// If any corresponding elided lifetimes appear in the output, we need to
/// replace them with references to the fresh name assigned to the corresponding
/// elided lifetime in the arguments.
///
/// For **Modern cases**, replace the anonymous parameter with a
/// reference to a specific freshly-named lifetime that was
/// introduced in argument
///
/// For **Dyn Bound** cases, pass responsibility to
/// `resole_lifetime` code.
Replace(LtReplacement),
}

/// The type of elided lifetime replacement to perform on `async fn` return types.
#[derive(Copy, Clone)]
enum LtReplacement {
/// Fresh name introduced by the single non-dyn elided lifetime
/// in the arguments of the async fn.
Some(ParamName),

/// There is no single non-dyn elided lifetime because no lifetimes
/// appeared in the arguments.
NoLifetimes,

/// There is no single non-dyn elided lifetime because multiple
/// lifetimes appeared in the arguments.
MultipleLifetimes,
}

/// Calculates the `LtReplacement` to use for elided lifetimes in the return
/// type based on the fresh elided lifetimes introduced in argument position.
fn get_elided_lt_replacement(arg_position_lifetimes: &[(Span, ParamName)]) -> LtReplacement {
match arg_position_lifetimes {
[] => LtReplacement::NoLifetimes,
[(_span, param)] => LtReplacement::Some(*param),
_ => LtReplacement::MultipleLifetimes,
}
}

struct ImplTraitTypeIdVisitor<'a> { ids: &'a mut SmallVec<[NodeId; 1]> }
Expand Down Expand Up @@ -2318,8 +2275,7 @@ impl<'a> LoweringContext<'a> {
err.emit();
}
AnonymousLifetimeMode::PassThrough |
AnonymousLifetimeMode::ReportError |
AnonymousLifetimeMode::Replace(_) => {
AnonymousLifetimeMode::ReportError => {
self.sess.buffer_lint_with_diagnostic(
ELIDED_LIFETIMES_IN_PATHS,
CRATE_NODE_ID,
Expand Down Expand Up @@ -2515,7 +2471,6 @@ impl<'a> LoweringContext<'a> {

// Remember how many lifetimes were already around so that we can
// only look at the lifetime parameters introduced by the arguments.
let lifetime_count_before_args = self.lifetimes_to_define.len();
let inputs = self.with_anonymous_lifetime_mode(lt_mode, |this| {
decl.inputs
.iter()
Expand All @@ -2530,16 +2485,10 @@ impl<'a> LoweringContext<'a> {
});

let output = if let Some(ret_id) = make_ret_async {
// Calculate the `LtReplacement` to use for any return-position elided
// lifetimes based on the elided lifetime parameters introduced in the args.
let lt_replacement = get_elided_lt_replacement(
&self.lifetimes_to_define[lifetime_count_before_args..]
);
self.lower_async_fn_ret_ty(
&decl.output,
in_band_ty_params.expect("`make_ret_async` but no `fn_def_id`").0,
ret_id,
lt_replacement,
)
} else {
match decl.output {
Expand Down Expand Up @@ -2604,7 +2553,6 @@ impl<'a> LoweringContext<'a> {
output: &FunctionRetTy,
fn_def_id: DefId,
opaque_ty_node_id: NodeId,
elided_lt_replacement: LtReplacement,
) -> hir::FunctionRetTy {
let span = output.span();

Expand All @@ -2622,9 +2570,18 @@ impl<'a> LoweringContext<'a> {

self.allocate_hir_id_counter(opaque_ty_node_id);

let input_lifetimes_count = self.in_scope_lifetimes.len() + self.lifetimes_to_define.len();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty clever and probably deserves a comment-- that in_scope_lifetimes contains all the non-elided lifetime names in scope, and lifetimes_to_define says how many lifetimes we have been paired with CreateParameter, so the last lifetime_params.len() - (in_scope_lifetimes + lifetimes_to_define) elements of lifetime_params are all elided lifetimes. Obviously the way I just worded it is probably even more confusing than just reading the code, but I think some explanation would be helpful.

let (opaque_ty_id, lifetime_params) = self.with_hir_id_owner(opaque_ty_node_id, |this| {
// We have to be careful to get elision right here. The
// idea is that we create a lifetime parameter for each
// lifetime in the return type. So, given a return type
// like `async fn foo(..) -> &[&u32]`, we lower to `impl
// Future<Output = &'1 [ &'2 u32 ]>`.
//
// Then, we will create `fn foo(..) -> Foo<'_, '_>`, and
// hence the elision takes place at the fn site.
let future_bound = this.with_anonymous_lifetime_mode(
AnonymousLifetimeMode::Replace(elided_lt_replacement),
AnonymousLifetimeMode::CreateParameter,
|this| this.lower_async_fn_output_type_to_future_bound(
output,
fn_def_id,
Expand Down Expand Up @@ -2678,19 +2635,53 @@ impl<'a> LoweringContext<'a> {
(opaque_ty_id, lifetime_params)
});

let generic_args =
lifetime_params
.iter().cloned()
.map(|(span, hir_name)| {
GenericArg::Lifetime(hir::Lifetime {
hir_id: self.next_id(),
span,
name: hir::LifetimeName::Param(hir_name),
})
// Create the generic lifetime arguments that we will supply
// to the opaque return type. Consider:
//
// ```rust
// async fn foo(x: &u32, ) -> &[&u32] { .. }
// ```
//
// Here, we would create something like:
//
// ```rust
// type Foo<'a, 'b, 'c> = impl Future<Output = &'a [&'b u32]>;
// fn foo<'a>(x: &'a u32) -> Foo<'a, '_, '_>
// ```
//
// Note that for the lifetimes which came from the input
// (`'a`, here), we supply them as arguments to the return
// type `Foo`. But for those lifetime parameters (`'b`, `'c`)
// that we created from the return type, we want to use `'_`
// in the return type, so as to trigger elision.
let mut generic_args: Vec<_> =
lifetime_params[..input_lifetimes_count]
.iter()
.map(|&(span, hir_name)| {
GenericArg::Lifetime(hir::Lifetime {
hir_id: self.next_id(),
span,
name: hir::LifetimeName::Param(hir_name),
})
.collect();
})
.collect();
generic_args.extend(
lifetime_params[input_lifetimes_count..]
.iter()
.map(|&(span, _)| {
GenericArg::Lifetime(hir::Lifetime {
hir_id: self.next_id(),
span,
name: hir::LifetimeName::Implicit,
})
})
);

let opaque_ty_ref = hir::TyKind::Def(hir::ItemId { id: opaque_ty_id }, generic_args);
// Create the `Foo<...>` refernece itself. Note that the `type
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved
// Foo = impl Trait` is, internally, created as a child of the
// async fn, so the *type parameters* are inherited. It's
// only the lifetime parameters that we must supply.
let opaque_ty_ref = hir::TyKind::Def(hir::ItemId { id: opaque_ty_id }, generic_args.into());

hir::FunctionRetTy::Return(P(hir::Ty {
node: opaque_ty_ref,
Expand Down Expand Up @@ -2786,11 +2777,6 @@ impl<'a> LoweringContext<'a> {
}

AnonymousLifetimeMode::ReportError => self.new_error_lifetime(Some(l.id), span),

AnonymousLifetimeMode::Replace(replacement) => {
let hir_id = self.lower_node_id(l.id);
self.replace_elided_lifetime(hir_id, span, replacement)
}
},
ident => {
self.maybe_collect_in_band_lifetime(ident);
Expand All @@ -2813,39 +2799,6 @@ impl<'a> LoweringContext<'a> {
}
}

/// Replace a return-position elided lifetime with the elided lifetime
/// from the arguments.
fn replace_elided_lifetime(
&mut self,
hir_id: hir::HirId,
span: Span,
replacement: LtReplacement,
) -> hir::Lifetime {
let multiple_or_none = match replacement {
LtReplacement::Some(name) => {
return hir::Lifetime {
hir_id,
span,
name: hir::LifetimeName::Param(name),
};
}
LtReplacement::MultipleLifetimes => "multiple",
LtReplacement::NoLifetimes => "none",
};

let mut err = crate::middle::resolve_lifetime::report_missing_lifetime_specifiers(
self.sess,
span,
1,
);
err.note(&format!(
"return-position elided lifetimes require exactly one \
input-position elided lifetime, found {}.", multiple_or_none));
err.emit();

hir::Lifetime { hir_id, span, name: hir::LifetimeName::Error }
}

fn lower_generic_params(
&mut self,
params: &[GenericParam],
Expand Down Expand Up @@ -5791,10 +5744,6 @@ impl<'a> LoweringContext<'a> {
AnonymousLifetimeMode::ReportError => self.new_error_lifetime(None, span),

AnonymousLifetimeMode::PassThrough => self.new_implicit_lifetime(span),

AnonymousLifetimeMode::Replace(replacement) => {
self.new_replacement_lifetime(replacement, span)
}
}
}

Expand Down Expand Up @@ -5848,10 +5797,6 @@ impl<'a> LoweringContext<'a> {
// This is the normal case.
AnonymousLifetimeMode::PassThrough => self.new_implicit_lifetime(span),

AnonymousLifetimeMode::Replace(replacement) => {
self.new_replacement_lifetime(replacement, span)
}

AnonymousLifetimeMode::ReportError => self.new_error_lifetime(None, span),
}
}
Expand Down Expand Up @@ -5883,25 +5828,11 @@ impl<'a> LoweringContext<'a> {

// This is the normal case.
AnonymousLifetimeMode::PassThrough => {}

// We don't need to do any replacement here as this lifetime
// doesn't refer to an elided lifetime elsewhere in the function
// signature.
AnonymousLifetimeMode::Replace(_) => {}
}

self.new_implicit_lifetime(span)
}

fn new_replacement_lifetime(
&mut self,
replacement: LtReplacement,
span: Span,
) -> hir::Lifetime {
let hir_id = self.next_id();
self.replace_elided_lifetime(hir_id, span, replacement)
}

fn new_implicit_lifetime(&mut self, span: Span) -> hir::Lifetime {
hir::Lifetime {
hir_id: self.next_id(),
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/async-await/issues/issue-63388-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// edition:2018

#![feature(async_await)]

struct Xyz {
a: u64,
}

trait Foo {}

impl Xyz {
async fn do_sth<'a>(
&'a self, foo: &dyn Foo
) -> &dyn Foo //~ ERROR lifetime mismatch
{
foo
}
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/async-await/issues/issue-63388-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0623]: lifetime mismatch
--> $DIR/issue-63388-1.rs:14:10
|
LL | &'a self, foo: &dyn Foo
| -------- this parameter and the return type are declared with different lifetimes...
LL | ) -> &dyn Foo
| ^^^^^^^^
| |
| ...but data from `foo` is returned here

error: aborting due to previous error

20 changes: 20 additions & 0 deletions src/test/ui/async-await/issues/issue-63388-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// edition:2018

#![feature(async_await)]

struct Xyz {
a: u64,
}

trait Foo {}

impl Xyz {
async fn do_sth<'a>(
foo: &dyn Foo, bar: &'a dyn Foo //~ ERROR cannot infer
) -> &dyn Foo //~ ERROR missing lifetime specifier
{
foo
}
}

fn main() {}
29 changes: 29 additions & 0 deletions src/test/ui/async-await/issues/issue-63388-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error[E0106]: missing lifetime specifier
--> $DIR/issue-63388-2.rs:14:10
|
LL | ) -> &dyn Foo
| ^ help: consider using the named lifetime: `&'a`
|
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from `foo` or `bar`

error: cannot infer an appropriate lifetime
--> $DIR/issue-63388-2.rs:13:9
|
LL | foo: &dyn Foo, bar: &'a dyn Foo
| ^^^ ...but this borrow...
LL | ) -> &dyn Foo
| -------- this return type evaluates to the `'static` lifetime...
|
note: ...can't outlive the lifetime '_ as defined on the method body at 13:14
--> $DIR/issue-63388-2.rs:13:14
|
LL | foo: &dyn Foo, bar: &'a dyn Foo
| ^
help: you can add a constraint to the return type to make it last less than `'static` and match the lifetime '_ as defined on the method body at 13:14
|
LL | ) -> &dyn Foo + '_
| ^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0106`.
19 changes: 19 additions & 0 deletions src/test/ui/async-await/issues/issue-63388-3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// edition:2018
// check-pass

#![feature(async_await)]

struct Xyz {
a: u64,
}

trait Foo {}

impl Xyz {
async fn do_sth(
&self, foo: &dyn Foo
) {
}
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/async-await/issues/issue-63388-4.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// check-pass
// edition:2018

#![feature(async_await)]

struct A;

impl A {
async fn foo(&self, f: &u32) -> &A { self }
}

fn main() { }
Loading