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

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

Merged
merged 7 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion src/librustc_middle/ty/context.rs
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
49 changes: 26 additions & 23 deletions src/librustc_trait_selection/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub trait InferCtxtExt<'tcx> {
err: &mut DiagnosticBuilder<'_>,
target_span: Span,
scope_span: &Option<Span>,
await_span: Span,
yield_span: Option<Span>,
expr: Option<hir::HirId>,
snippet: String,
inner_generator_body: Option<&hir::Body<'_>>,
Expand Down Expand Up @@ -1353,7 +1353,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
err: &mut DiagnosticBuilder<'_>,
target_span: Span,
scope_span: &Option<Span>,
yield_span: Span,
yield_span: Option<Span>,
expr: Option<hir::HirId>,
snippet: String,
inner_generator_body: Option<&hir::Body<'_>>,
Expand Down Expand Up @@ -1446,32 +1446,35 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
"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),
);

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);
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),
target_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 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),
);
}

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 {
Expand Down
36 changes: 32 additions & 4 deletions src/librustc_typeck/check/generator_interior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_span::Span;

struct InteriorVisitor<'a, 'tcx> {
fcx: &'a FnCtxt<'a, 'tcx>,
closure_def_id: DefId,
types: FxHashMap<ty::GeneratorInteriorTypeCause<'tcx>, usize>,
region_scope_tree: &'tcx region::ScopeTree,
expr_count: usize,
Expand All @@ -30,6 +31,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
scope: Option<region::Scope>,
expr: Option<&'tcx Expr<'tcx>>,
source_span: Span,
is_upvar: bool,
) {
use rustc_span::DUMMY_SP;

Expand Down Expand Up @@ -96,7 +98,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 All @@ -117,6 +119,20 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {
unresolved_type, unresolved_type_span
);
self.prev_unresolved_span = unresolved_type_span;
csmoe marked this conversation as resolved.
Show resolved Hide resolved
} else {
csmoe marked this conversation as resolved.
Show resolved Hide resolved
if is_upvar {
let entries = self.types.len();
let scope_span = scope.map(|s| s.span(self.fcx.tcx, self.region_scope_tree));
self.types
.entry(ty::GeneratorInteriorTypeCause {
span: source_span,
ty: &ty,
scope_span,
yield_span: None,
expr: expr.map(|e| e.hir_id),
})
.or_insert(entries);
}
}
}
}
Expand All @@ -130,8 +146,12 @@ pub fn resolve_interior<'a, 'tcx>(
kind: hir::GeneratorKind,
) {
let body = fcx.tcx.hir().body(body_id);

csmoe marked this conversation as resolved.
Show resolved Hide resolved
let closure_def_id = fcx.tcx.hir().body_owner_def_id(body_id).to_def_id();

let mut visitor = InteriorVisitor {
fcx,
closure_def_id,
types: FxHashMap::default(),
region_scope_tree: fcx.tcx.region_scope_tree(def_id),
expr_count: 0,
Expand Down Expand Up @@ -223,7 +243,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
if let PatKind::Binding(..) = pat.kind {
let scope = self.region_scope_tree.var_scope(pat.hir_id.local_id);
let ty = self.fcx.tables.borrow().pat_ty(pat);
self.record(ty, Some(scope), None, pat.span);
self.record(ty, Some(scope), None, pat.span, false);
}
}

Expand Down Expand Up @@ -264,7 +284,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
// If there are adjustments, then record the final type --
// this is the actual value that is being produced.
if let Some(adjusted_ty) = self.fcx.tables.borrow().expr_ty_adjusted_opt(expr) {
self.record(adjusted_ty, scope, Some(expr), expr.span);
self.record(adjusted_ty, scope, Some(expr), expr.span, false);
}

// Also record the unadjusted type (which is the only type if
Expand Down Expand Up @@ -292,9 +312,17 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> {
// The type table might not have information for this expression
// if it is in a malformed scope. (#66387)
if let Some(ty) = self.fcx.tables.borrow().expr_ty_opt(expr) {
self.record(ty, scope, Some(expr), expr.span);
self.record(ty, scope, Some(expr), expr.span, false);
} else {
self.fcx.tcx.sess.delay_span_bug(expr.span, "no type for node");
}

if let Some(upvars) = self.fcx.tcx.upvars(self.closure_def_id) {
csmoe marked this conversation as resolved.
Show resolved Hide resolved
for (upvar_id, upvar) in upvars.iter() {
let upvar_ty = self.fcx.tables.borrow().node_type(*upvar_id);
debug!("type of upvar: {:?}", upvar_ty);
self.record(upvar_ty, scope, Some(expr), upvar.span, true);
}
}
}
}
7 changes: 7 additions & 0 deletions src/test/ui/async-await/issue-70818.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// edition 2018
csmoe marked this conversation as resolved.
Show resolved Hide resolved

fn foo<T: Sized>(ty: T) -> impl std::future::Future<Output = T> + Send { //~ Error `T` cannot be sent between threads safely
csmoe marked this conversation as resolved.
Show resolved Hide resolved
async { ty }
}

fn main() {}