Skip to content

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

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

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 6 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
2 changes: 1 addition & 1 deletion src/librustc_middle/ty/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ pub struct GeneratorInteriorTypeCause<'tcx> {
/// Span of the scope of the captured binding.
pub scope_span: Option<Span>,
/// Span of `.await` or `yield` expression.
pub yield_span: Span,
pub yield_span: Option<Span>,
/// Expr which the type evaluated from.
pub expr: Option<hir::HirId>,
}
Expand Down
242 changes: 128 additions & 114 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 @@ -125,19 +125,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: Option<(Span, Option<Span>, Option<Span>, Option<hir::HirId>, Option<Span>)>,
upvar: Option<(Ty<'tcx>, 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 +1132,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 +1280,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
);
eq
};
let target_span = tables
let interior = tables
.generator_interior_types
.iter()
.find(|ty::GeneratorInteriorTypeCause { ty, .. }| ty_matches(ty))
Expand All @@ -1306,39 +1301,36 @@ 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,
)
(*span, *scope_span, *yield_span, *expr, from_awaited_ty)
});

let upvar = if let Some(upvars) = self.tcx.upvars(generator_did) {
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((upvar_ty, upvar.span)) } else { None }
})
} else {
None
};

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={:?} \
generator_interior_types={:?} upvar: {:?}",
interior, tables.generator_interior_types, upvar
);
if let Some((target_span, Ok(snippet), scope_span, yield_span, expr, from_awaited_ty)) =
target_span
{
if interior.is_some() || upvar.is_some() {
self.note_obligation_cause_for_async_await(
err,
*target_span,
scope_span,
*yield_span,
*expr,
snippet,
interior,
upvar,
generator_body,
outer_generator,
trait_ref,
target_ty,
tables,
obligation,
next_code,
from_awaited_ty,
);
true
} else {
Expand All @@ -1351,19 +1343,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: Option<(Span, Option<Span>, Option<Span>, Option<hir::HirId>, Option<Span>)>,
upvar: Option<(Ty<'tcx>, 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,96 +1412,122 @@ 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);
if let Some((target_span, scope_span, yield_span, expr, from_awaited_ty)) = interior {
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),
);
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 span = MultiSpan::from_span(yield_span);
span.push_span_label(
yield_span,
format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet),
);
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
);

span.push_span_label(
target_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
if let Some(yield_span) = yield_span {
let mut span = MultiSpan::from_span(yield_span);
if let Ok(snippet) = source_map.span_to_snippet(target_span) {
span.push_span_label(
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(
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 {
err.span_note(
span,
&format!(
"{} {} as this value is used across {}",
future_or_generator, trait_explanation, an_await_or_yield
),
);
}
}
if let Some((_, upvar_span)) = upvar {
let mut span = MultiSpan::from_span(upvar_span);
span.push_span_label(
source_map.end_point(*scope_span),
format!("`{}` is later dropped here", snippet),
upvar_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
}

err.span_note(
span,
&format!(
"{} {} as this value is used across {}",
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,
};

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",
);
);
}
}
}
} else {
if let Some((_, upvar_span)) = upvar {
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 outer value {}", trait_explanation));
}
}

// Add a note for the item obligation that remains - normally a note pointing to the
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_typeck/check/generator_interior.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
span: source_span,
ty: &ty,
scope_span,
yield_span: yield_data.span,
yield_span: Some(yield_data.span),
expr: expr.map(|e| e.hir_id),
})
.or_insert(entries);
Expand Down Expand Up @@ -130,6 +130,7 @@ pub fn resolve_interior<'a, 'tcx>(
kind: hir::GeneratorKind,
) {
let body = fcx.tcx.hir().body(body_id);

let mut visitor = InteriorVisitor {
fcx,
types: FxHashMap::default(),
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

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 outer 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