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

Make it more clear what an about async fn's returns when referring to what it returns #76765

Merged
merged 3 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,43 +102,89 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
None => String::new(),
};

let (span_1, span_2, main_label, span_label) = match (sup_is_ret_type, sub_is_ret_type) {
(None, None) => {
let (main_label_1, span_label_1) = if ty_sup.hir_id == ty_sub.hir_id {
let (span_1, span_2, main_label, span_label, future_return_type) =
match (sup_is_ret_type, sub_is_ret_type) {
(None, None) => {
let (main_label_1, span_label_1) = if ty_sup.hir_id == ty_sub.hir_id {
(
"this type is declared with multiple lifetimes...".to_owned(),
"...but data with one lifetime flows into the other here".to_owned(),
)
} else {
(
"these two types are declared with different lifetimes...".to_owned(),
format!("...but data{} flows{} here", span_label_var1, span_label_var2),
)
};
(ty_sup.span, ty_sub.span, main_label_1, span_label_1, None)
}

(Some(ret_span), _) => {
let sup_future = self.future_return_type(scope_def_id_sup);
let (return_type, action) = if let Some(_) = sup_future {
("returned future", "held across an await point")
} else {
("return type", "returned")
};

(
"this type is declared with multiple lifetimes...".to_owned(),
"...but data with one lifetime flows into the other here".to_owned(),
ty_sub.span,
ret_span,
format!(
"this parameter and the {} are declared with different lifetimes...",
return_type
),
format!("...but data{} is {} here", span_label_var1, action),
sup_future,
)
} else {
}
(_, Some(ret_span)) => {
let sub_future = self.future_return_type(scope_def_id_sub);
let (return_type, action) = if let Some(_) = sub_future {
("returned future", "held across an await point")
} else {
("return type", "returned")
};

(
"these two types are declared with different lifetimes...".to_owned(),
format!("...but data{} flows{} here", span_label_var1, span_label_var2),
ty_sup.span,
ret_span,
format!(
"this parameter and the {} are declared with different lifetimes...",
return_type
),
format!("...but data{} is {} here", span_label_var1, action),
sub_future,
)
};
(ty_sup.span, ty_sub.span, main_label_1, span_label_1)
}

(Some(ret_span), _) => (
ty_sub.span,
ret_span,
"this parameter and the return type are declared with different lifetimes..."
.to_owned(),
format!("...but data{} is returned here", span_label_var1),
),
(_, Some(ret_span)) => (
ty_sup.span,
ret_span,
"this parameter and the return type are declared with different lifetimes..."
.to_owned(),
format!("...but data{} is returned here", span_label_var1),
),
};

struct_span_err!(self.tcx().sess, span, E0623, "lifetime mismatch")
.span_label(span_1, main_label)
.span_label(span_2, String::new())
.span_label(span, span_label)
.emit();
}
};

let mut e = struct_span_err!(self.tcx().sess, span, E0623, "lifetime mismatch");

e.span_label(span_1, main_label);
e.span_label(span_2, String::new());
e.span_label(span, span_label);

if let Some(t) = future_return_type {
let snip = self
.tcx()
.sess
.source_map()
.span_to_snippet(t.span)
.ok()
.and_then(|s| match (&t.kind, s.as_str()) {
(rustc_hir::TyKind::Tup(&[]), "") => Some("()".to_string()),
(_, "") => None,
_ => Some(s),
})
.unwrap_or("{unnamed_type}".to_string());

e.span_label(
t.span,
&format!("this `async fn` implicitly returns an `impl Future<Output = {}>`", snip),
);
}
e.emit();
Some(ErrorReported)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,60 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
})
}

pub(super) fn future_return_type(
Copy link
Member

Choose a reason for hiding this comment

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

cc #78755

&self,
local_def_id: LocalDefId,
) -> Option<&rustc_hir::Ty<'_>> {
if let Some(hir::IsAsync::Async) = self.asyncness(local_def_id) {
if let rustc_middle::ty::Opaque(def_id, _) =
self.tcx().type_of(local_def_id).fn_sig(self.tcx()).output().skip_binder().kind()
{
match self.tcx().hir().get_if_local(*def_id) {
Some(hir::Node::Item(hir::Item {
kind:
hir::ItemKind::OpaqueTy(hir::OpaqueTy {
bounds,
origin: hir::OpaqueTyOrigin::AsyncFn,
..
}),
..
})) => {
for b in bounds.iter() {
if let hir::GenericBound::LangItemTrait(
hir::LangItem::Future,
_span,
_hir_id,
generic_args,
) = b
{
for type_binding in generic_args.bindings.iter() {
if type_binding.ident.name == rustc_span::sym::Output {
if let hir::TypeBindingKind::Equality { ty } =
type_binding.kind
{
return Some(ty);
}
}
}
}
}
}
_ => {}
}
}
}
None
}

pub(super) fn asyncness(&self, local_def_id: LocalDefId) -> Option<hir::IsAsync> {
// similar to the asyncness fn in rustc_ty::ty
let hir_id = self.tcx().hir().local_def_id_to_hir_id(local_def_id);
let node = self.tcx().hir().get(hir_id);
let fn_like = rustc_middle::hir::map::blocks::FnLikeNode::from_node(node)?;

Some(fn_like.asyncness())
}

// Here, we check for the case where the anonymous region
// is in the return type.
// FIXME(#42703) - Need to handle certain cases here.
Expand Down
6 changes: 4 additions & 2 deletions src/test/ui/async-await/issues/issue-63388-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ error[E0623]: lifetime mismatch
--> $DIR/issue-63388-1.rs:14:9
|
LL | &'a self, foo: &dyn Foo
| -------- this parameter and the return type are declared with different lifetimes...
| -------- this parameter and the returned future are declared with different lifetimes...
LL | ) -> &dyn Foo
| --------
| |
| this `async fn` implicitly returns an `impl Future<Output = &dyn Foo>`
LL | {
LL | foo
| ^^^ ...but data from `foo` is returned here
| ^^^ ...but data from `foo` is held across an await point here

error: aborting due to previous error

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ error[E0623]: lifetime mismatch
LL | async fn async_ret_impl_trait1<'a, 'b>(a: &'a u8, b: &'b u8) -> impl Trait<'a> {
| ------ ^^^^^^^^^^^^^^
| | |
| | ...but data from `b` is returned here
| this parameter and the return type are declared with different lifetimes...
| | ...but data from `b` is held across an await point here
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this wording doesn't really work if the error is in the function signature itself. I'm confused why the span is pointing here, but one way to make the diagnostic less wrong is to remove the word "here". Then we're just telling you what's happening and providing a span that might help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, when I find time (hopefully tomorrow) I will dig into these comments!

| | this `async fn` implicitly returns an `impl Future<Output = impl Trait<'a>>`
| this parameter and the returned future are declared with different lifetimes...

error: aborting due to previous error

Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/issues/issue-76547.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: lifetime may not live long enough
--> $DIR/issue-76547.rs:19:14
|
LL | async fn fut(bufs: &mut [&mut [u8]]) {
| ^^^^ - - let's call the lifetime of this reference `'2`
| | |
| | let's call the lifetime of this reference `'1`
| assignment requires that `'1` must outlive `'2`

error: lifetime may not live long enough
--> $DIR/issue-76547.rs:33:15
|
LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
| ^^^^ - - let's call the lifetime of this reference `'2`
| | |
| | let's call the lifetime of this reference `'1`
| assignment requires that `'1` must outlive `'2`

error: aborting due to 2 previous errors

38 changes: 38 additions & 0 deletions src/test/ui/issues/issue-76547.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Test for for diagnostic improvement issue #76547
tmandry marked this conversation as resolved.
Show resolved Hide resolved
// edition:2018

use std::{
future::Future,
task::{Context, Poll}
};
use std::pin::Pin;

pub struct ListFut<'a>(&'a mut [&'a mut [u8]]);
impl<'a> Future for ListFut<'a> {
type Output = ();

fn poll(self: Pin<&mut Self>, _cx: &mut Context) -> Poll<Self::Output> {
unimplemented!()
}
}

async fn fut(bufs: &mut [&mut [u8]]) {
ListFut(bufs).await
//~^ ERROR lifetime mismatch
}

pub struct ListFut2<'a>(&'a mut [&'a mut [u8]]);
impl<'a> Future for ListFut2<'a> {
type Output = i32;

fn poll(self: Pin<&mut Self>, _cx: &mut Context) -> Poll<Self::Output> {
unimplemented!()
}
}

async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
ListFut2(bufs).await
//~^ ERROR lifetime mismatch
}

fn main() {}
25 changes: 25 additions & 0 deletions src/test/ui/issues/issue-76547.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0623]: lifetime mismatch
--> $DIR/issue-76547.rs:20:13
|
LL | async fn fut(bufs: &mut [&mut [u8]]) {
| --------- -
| | |
| | this `async fn` implicitly returns an `impl Future<Output = ()>`
| this parameter and the returned future are declared with different lifetimes...
LL | ListFut(bufs).await
| ^^^^ ...but data from `bufs` is held across an await point here

error[E0623]: lifetime mismatch
--> $DIR/issue-76547.rs:34:14
|
LL | async fn fut2(bufs: &mut [&mut [u8]]) -> i32 {
| --------- ---
| | |
| | this `async fn` implicitly returns an `impl Future<Output = i32>`
| this parameter and the returned future are declared with different lifetimes...
LL | ListFut2(bufs).await
| ^^^^ ...but data from `bufs` is held across an await point here

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0623`.
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,28 @@ error[E0623]: lifetime mismatch
--> $DIR/arbitrary_self_types_pin_lifetime_mismatch-async.rs:8:52
|
LL | async fn a(self: Pin<&Foo>, f: &Foo) -> &Foo { f }
| ---- ---- ^ ...but data from `f` is returned here
| |
| this parameter and the return type are declared with different lifetimes...
| ---- ---- ^ ...but data from `f` is held across an await point here
| | |
| | this `async fn` implicitly returns an `impl Future<Output = &Foo>`
| this parameter and the returned future are declared with different lifetimes...

error[E0623]: lifetime mismatch
--> $DIR/arbitrary_self_types_pin_lifetime_mismatch-async.rs:11:82
|
LL | async fn c(self: Pin<&Self>, f: &Foo, g: &Foo) -> (Pin<&Foo>, &Foo) { (self, f) }
| ----- ----------------- ^ ...but data from `f` is returned here
| |
| this parameter and the return type are declared with different lifetimes...
| ----- ----------------- ^ ...but data from `f` is held across an await point here
| | |
| | this `async fn` implicitly returns an `impl Future<Output = (Pin<&Foo>, &Foo)>`
| this parameter and the returned future are declared with different lifetimes...

error[E0623]: lifetime mismatch
--> $DIR/arbitrary_self_types_pin_lifetime_mismatch-async.rs:17:64
|
LL | async fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> &() { arg }
| ----- --- ^^^ ...but data from `arg` is returned here
| |
| this parameter and the return type are declared with different lifetimes...
| ----- --- ^^^ ...but data from `arg` is held across an await point here
| | |
| | this `async fn` implicitly returns an `impl Future<Output = &()>`
| this parameter and the returned future are declared with different lifetimes...

error: aborting due to 3 previous errors

Expand Down
Loading