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.

Sign up for GitHub

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

Jump to bottom

Check non-Send/Sync upvars captured by generator #71923

Merged
merged 7 commits into from
May 20, 2020
Merged

Check non-Send/Sync upvars captured by generator #71923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e07f63e
add testcase for issue-70818
csmoe Apr 30, 2020
fb81c42
make yield span optional
csmoe May 5, 2020
a5d103f
record upvar into GeneratorInteriorTypeCause
csmoe May 5, 2020
d4bdcfd
don't record upvars into generator interior
csmoe May 6, 2020
382a963
filter upvars that cause trait obligation
csmoe May 6, 2020
d2e5a8e
bless issue-70818 test case
csmoe May 6, 2020
c9e1465
checking on either interior or upvar
csmoe May 7, 2020
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
243 changes: 135 additions & 108 deletions src/librustc_trait_selection/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ use std::fmt;
use super::InferCtxtPrivExt;
use crate::traits::query::evaluate_obligation::InferCtxtExt as _;

#[derive(Debug)]
pub enum GeneratorInteriorOrUpvar {
// span of interior type
Interior(Span),
// span of upvar
Upvar(Span),
}

// This trait is public to expose the diagnostics methods to clippy.
pub trait InferCtxtExt<'tcx> {
fn suggest_restricting_param_bound(
Expand Down Expand Up @@ -125,19 +133,15 @@ pub trait InferCtxtExt<'tcx> {
fn note_obligation_cause_for_async_await(
&self,
err: &mut DiagnosticBuilder<'_>,
target_span: Span,
scope_span: &Option<Span>,
await_span: Span,
expr: Option<hir::HirId>,
snippet: String,
interior_or_upvar_span: GeneratorInteriorOrUpvar,
interior_extra_info: Option<(Option<Span>, Span, Option<hir::HirId>, Option<Span>)>,
inner_generator_body: Option<&hir::Body<'_>>,
outer_generator: Option<DefId>,
trait_ref: ty::TraitRef<'_>,
target_ty: Ty<'tcx>,
tables: &ty::TypeckTables<'_>,
obligation: &PredicateObligation<'tcx>,
next_code: Option<&ObligationCauseCode<'tcx>>,
from_awaited_ty: Option<Span>,
);

fn note_obligation_cause_code<T>(
Expand Down Expand Up @@ -1136,7 +1140,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation.cause.span={:?}",
obligation.predicate, obligation.cause.span
);
let source_map = self.tcx.sess.source_map();
let hir = self.tcx.hir();

// Attempt to detect an async-await error by looking at the obligation causes, looking
Expand Down Expand Up @@ -1285,7 +1288,23 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
);
eq
};
let target_span = tables

let mut interior_or_upvar_span = None;
let mut interior_extra_info = None;

if let Some(upvars) = self.tcx.upvars(generator_did) {
interior_or_upvar_span = upvars.iter().find_map(|(upvar_id, upvar)| {
let upvar_ty = tables.node_type(*upvar_id);
let upvar_ty = self.resolve_vars_if_possible(&upvar_ty);
if ty_matches(&upvar_ty) {
Some(GeneratorInteriorOrUpvar::Upvar(upvar.span))
} else {
None
}
});
};

tables
.generator_interior_types
.iter()
.find(|ty::GeneratorInteriorTypeCause { ty, .. }| ty_matches(ty))
Expand All @@ -1306,39 +1325,28 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
.map(|expr| expr.span);
let ty::GeneratorInteriorTypeCause { span, scope_span, yield_span, expr, .. } =
cause;
(
span,
source_map.span_to_snippet(*span),
scope_span,
yield_span,
expr,
from_awaited_ty,
)

interior_or_upvar_span = Some(GeneratorInteriorOrUpvar::Interior(*span));
interior_extra_info = Some((*scope_span, *yield_span, *expr, from_awaited_ty));
});

debug!(
"maybe_note_obligation_cause_for_async_await: target_ty={:?} \
generator_interior_types={:?} target_span={:?}",
target_ty, tables.generator_interior_types, target_span
"maybe_note_obligation_cause_for_async_await: interior_or_upvar={:?} \
generator_interior_types={:?}",
interior_or_upvar_span, tables.generator_interior_types
);
if let Some((target_span, Ok(snippet), scope_span, yield_span, expr, from_awaited_ty)) =
target_span
{
if let Some(interior_or_upvar_span) = interior_or_upvar_span {
self.note_obligation_cause_for_async_await(
err,
*target_span,
scope_span,
*yield_span,
*expr,
snippet,
interior_or_upvar_span,
interior_extra_info,
generator_body,
outer_generator,
trait_ref,
target_ty,
tables,
obligation,
next_code,
from_awaited_ty,
);
true
} else {
Expand All @@ -1351,19 +1359,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
fn note_obligation_cause_for_async_await(
&self,
err: &mut DiagnosticBuilder<'_>,
target_span: Span,
scope_span: &Option<Span>,
yield_span: Span,
expr: Option<hir::HirId>,
snippet: String,
interior_or_upvar_span: GeneratorInteriorOrUpvar,
interior_extra_info: Option<(Option<Span>, Span, Option<hir::HirId>, Option<Span>)>,
inner_generator_body: Option<&hir::Body<'_>>,
outer_generator: Option<DefId>,
trait_ref: ty::TraitRef<'_>,
target_ty: Ty<'tcx>,
tables: &ty::TypeckTables<'_>,
obligation: &PredicateObligation<'tcx>,
next_code: Option<&ObligationCauseCode<'tcx>>,
from_awaited_ty: Option<Span>,
) {
let source_map = self.tcx.sess.source_map();

Expand Down Expand Up @@ -1424,46 +1428,27 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
format!("does not implement `{}`", trait_ref.print_only_trait_path())
};

if let Some(await_span) = from_awaited_ty {
// The type causing this obligation is one being awaited at await_span.
let mut span = MultiSpan::from_span(await_span);

span.push_span_label(
await_span,
format!("await occurs here on type `{}`, which {}", target_ty, trait_explanation),
);

err.span_note(
span,
&format!(
"future {not_trait} as it awaits another future which {not_trait}",
not_trait = trait_explanation
),
);
} else {
// Look at the last interior type to get a span for the `.await`.
debug!(
"note_obligation_cause_for_async_await generator_interior_types: {:#?}",
tables.generator_interior_types
);
let mut explain_yield = |interior_span: Span,
yield_span: Span,
scope_span: Option<Span>| {
let mut span = MultiSpan::from_span(yield_span);
span.push_span_label(
yield_span,
format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet),
);

span.push_span_label(
target_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);

// If available, use the scope span to annotate the drop location.
if let Some(scope_span) = scope_span {
if let Ok(snippet) = source_map.span_to_snippet(interior_span) {
span.push_span_label(
source_map.end_point(*scope_span),
format!("`{}` is later dropped here", snippet),
yield_span,
format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet),
);
// If available, use the scope span to annotate the drop location.
if let Some(scope_span) = scope_span {
span.push_span_label(
source_map.end_point(scope_span),
format!("`{}` is later dropped here", snippet),
);
}
}
span.push_span_label(
interior_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);

err.span_note(
span,
Expand All @@ -1472,48 +1457,90 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
future_or_generator, trait_explanation, an_await_or_yield
),
);
}

if let Some(expr_id) = expr {
let expr = hir.expect_expr(expr_id);
debug!("target_ty evaluated from {:?}", expr);

let parent = hir.get_parent_node(expr_id);
if let Some(hir::Node::Expr(e)) = hir.find(parent) {
let parent_span = hir.span(parent);
let parent_did = parent.owner.to_def_id();
// ```rust
// impl T {
// fn foo(&self) -> i32 {}
// }
// T.foo();
// ^^^^^^^ a temporary `&T` created inside this method call due to `&self`
// ```
//
let is_region_borrow =
tables.expr_adjustments(expr).iter().any(|adj| adj.is_region_borrow());

// ```rust
// struct Foo(*const u8);
// bar(Foo(std::ptr::null())).await;
// ^^^^^^^^^^^^^^^^^^^^^ raw-ptr `*T` created inside this struct ctor.
// ```
debug!("parent_def_kind: {:?}", self.tcx.def_kind(parent_did));
let is_raw_borrow_inside_fn_like_call = match self.tcx.def_kind(parent_did) {
DefKind::Fn | DefKind::Ctor(..) => target_ty.is_unsafe_ptr(),
_ => false,
};
};
match interior_or_upvar_span {
GeneratorInteriorOrUpvar::Interior(interior_span) => {
if let Some((scope_span, yield_span, expr, from_awaited_ty)) = interior_extra_info {
if let Some(await_span) = from_awaited_ty {
// The type causing this obligation is one being awaited at await_span.
let mut span = MultiSpan::from_span(await_span);
span.push_span_label(
await_span,
format!(
"await occurs here on type `{}`, which {}",
target_ty, trait_explanation
),
);
err.span_note(
span,
&format!(
"future {not_trait} as it awaits another future which {not_trait}",
not_trait = trait_explanation
),
);
} else {
// Look at the last interior type to get a span for the `.await`.
debug!(
"note_obligation_cause_for_async_await generator_interior_types: {:#?}",
tables.generator_interior_types
);
explain_yield(interior_span, yield_span, scope_span);
}

if (tables.is_method_call(e) && is_region_borrow)
|| is_raw_borrow_inside_fn_like_call
{
err.span_help(
parent_span,
"consider moving this into a `let` \
if let Some(expr_id) = expr {
let expr = hir.expect_expr(expr_id);
debug!("target_ty evaluated from {:?}", expr);

let parent = hir.get_parent_node(expr_id);
if let Some(hir::Node::Expr(e)) = hir.find(parent) {
let parent_span = hir.span(parent);
let parent_did = parent.owner.to_def_id();
// ```rust
// impl T {
// fn foo(&self) -> i32 {}
// }
// T.foo();
// ^^^^^^^ a temporary `&T` created inside this method call due to `&self`
// ```
//
let is_region_borrow = tables
.expr_adjustments(expr)
.iter()
.any(|adj| adj.is_region_borrow());

// ```rust
// struct Foo(*const u8);
// bar(Foo(std::ptr::null())).await;
// ^^^^^^^^^^^^^^^^^^^^^ raw-ptr `*T` created inside this struct ctor.
// ```
debug!("parent_def_kind: {:?}", self.tcx.def_kind(parent_did));
let is_raw_borrow_inside_fn_like_call =
match self.tcx.def_kind(parent_did) {
DefKind::Fn | DefKind::Ctor(..) => target_ty.is_unsafe_ptr(),
_ => false,
};

if (tables.is_method_call(e) && is_region_borrow)
|| is_raw_borrow_inside_fn_like_call
{
err.span_help(
parent_span,
"consider moving this into a `let` \
binding to create a shorter lived borrow",
);
);
}
}
}
}
}
GeneratorInteriorOrUpvar::Upvar(upvar_span) => {
let mut span = MultiSpan::from_span(upvar_span);
span.push_span_label(
upvar_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
err.span_note(span, &format!("captured value {}", trait_explanation));
}
}

// Add a note for the item obligation that remains - normally a note pointing to the
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/async-await/issue-70818.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// edition:2018
csmoe marked this conversation as resolved.
Show resolved Hide resolved

use std::future::Future;
fn foo<T: Send, U>(ty: T, ty1: U) -> impl Future<Output = (T, U)> + Send {
//~^ Error future cannot be sent between threads safely
async { (ty, ty1) }
}

fn main() {}
23 changes: 23 additions & 0 deletions src/test/ui/async-await/issue-70818.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: future cannot be sent between threads safely
--> $DIR/issue-70818.rs:4:38
|
LL | fn foo<T: Send, U>(ty: T, ty1: U) -> impl Future<Output = (T, U)> + Send {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
LL |
LL | async { (ty, ty1) }
| ------------------- this returned value is of type `impl std::future::Future`
|
= help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `U`
note: captured value is not `Send`
--> $DIR/issue-70818.rs:6:18
|
LL | async { (ty, ty1) }
| ^^^ has type `U` which is not `Send`
= note: the return type of a function must have a statically known size
help: consider restricting type parameter `U`
|
LL | fn foo<T: Send, U: std::marker::Send>(ty: T, ty1: U) -> impl Future<Output = (T, U)> + Send {
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error