From e07f63e79a618a0c5e53a71d053da0fa0dbc6970 Mon Sep 17 00:00:00 2001 From: csmoe Date: Thu, 30 Apr 2020 12:15:59 +0800 Subject: [PATCH 1/7] add testcase for issue-70818 --- src/test/ui/async-await/issue-70818.rs | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 src/test/ui/async-await/issue-70818.rs diff --git a/src/test/ui/async-await/issue-70818.rs b/src/test/ui/async-await/issue-70818.rs new file mode 100644 index 0000000000000..b914c9b17abdd --- /dev/null +++ b/src/test/ui/async-await/issue-70818.rs @@ -0,0 +1,5 @@ +// edition 2018 + +fn d(t: T) -> impl std::future::Future + Send { //~ Error `T` cannot be sent between threads safely + async { t } +} From fb81c429ebd45f9ba2b1810f548cf59a45feb222 Mon Sep 17 00:00:00 2001 From: csmoe Date: Tue, 5 May 2020 23:24:34 +0800 Subject: [PATCH 2/7] make yield span optional --- src/librustc_middle/ty/context.rs | 2 +- .../traits/error_reporting/suggestions.rs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index e43eb01ad96f7..cc89e59096d89 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -305,7 +305,7 @@ pub struct GeneratorInteriorTypeCause<'tcx> { /// Span of the scope of the captured binding. pub scope_span: Option, /// Span of `.await` or `yield` expression. - pub yield_span: Span, + pub yield_span: Option, /// Expr which the type evaluated from. pub expr: Option, } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 5ec2d68ab2a7d..cd1c75dc5f291 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -127,7 +127,7 @@ pub trait InferCtxtExt<'tcx> { err: &mut DiagnosticBuilder<'_>, target_span: Span, scope_span: &Option, - await_span: Span, + yield_span: Option, expr: Option, snippet: String, inner_generator_body: Option<&hir::Body<'_>>, @@ -1353,7 +1353,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, target_span: Span, scope_span: &Option, - yield_span: Span, + yield_span: Option, expr: Option, snippet: String, inner_generator_body: Option<&hir::Body<'_>>, @@ -1446,11 +1446,13 @@ 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), - ); + + 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), + ); span.push_span_label( target_span, From a5d103ff90f4ad90b1f13f5b5de83a3eb758d9e2 Mon Sep 17 00:00:00 2001 From: csmoe Date: Tue, 5 May 2020 23:26:33 +0800 Subject: [PATCH 3/7] record upvar into GeneratorInteriorTypeCause --- .../traits/error_reporting/suggestions.rs | 35 +++++++++--------- .../check/generator_interior.rs | 36 ++++++++++++++++--- src/test/ui/async-await/issue-70818.rs | 6 ++-- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index cd1c75dc5f291..9525910e39c6f 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1454,26 +1454,27 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { 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 { 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 { diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index ce376a08ea604..aa4abcd722426 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -16,6 +16,7 @@ use rustc_span::Span; struct InteriorVisitor<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, + closure_def_id: DefId, types: FxHashMap, usize>, region_scope_tree: &'tcx region::ScopeTree, expr_count: usize, @@ -30,6 +31,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { scope: Option, expr: Option<&'tcx Expr<'tcx>>, source_span: Span, + is_upvar: bool, ) { use rustc_span::DUMMY_SP; @@ -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); @@ -117,6 +119,20 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { unresolved_type, unresolved_type_span ); self.prev_unresolved_span = unresolved_type_span; + } else { + 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); + } } } } @@ -130,8 +146,12 @@ pub fn resolve_interior<'a, 'tcx>( kind: hir::GeneratorKind, ) { let body = fcx.tcx.hir().body(body_id); + + 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, @@ -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); } } @@ -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 @@ -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) { + 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); + } + } } } diff --git a/src/test/ui/async-await/issue-70818.rs b/src/test/ui/async-await/issue-70818.rs index b914c9b17abdd..9bbaacd2f11b9 100644 --- a/src/test/ui/async-await/issue-70818.rs +++ b/src/test/ui/async-await/issue-70818.rs @@ -1,5 +1,7 @@ // edition 2018 -fn d(t: T) -> impl std::future::Future + Send { //~ Error `T` cannot be sent between threads safely - async { t } +fn foo(ty: T) -> impl std::future::Future + Send { //~ Error `T` cannot be sent between threads safely + async { ty } } + +fn main() {} From d4bdcfd3cfbfa025c5363149a14f2b36a584b54a Mon Sep 17 00:00:00 2001 From: csmoe Date: Wed, 6 May 2020 16:46:37 +0800 Subject: [PATCH 4/7] don't record upvars into generator interior --- .../check/generator_interior.rs | 33 ++----------------- 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index aa4abcd722426..7a5f74384f8c3 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -16,7 +16,6 @@ use rustc_span::Span; struct InteriorVisitor<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, - closure_def_id: DefId, types: FxHashMap, usize>, region_scope_tree: &'tcx region::ScopeTree, expr_count: usize, @@ -31,7 +30,6 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { scope: Option, expr: Option<&'tcx Expr<'tcx>>, source_span: Span, - is_upvar: bool, ) { use rustc_span::DUMMY_SP; @@ -119,20 +117,6 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { unresolved_type, unresolved_type_span ); self.prev_unresolved_span = unresolved_type_span; - } else { - 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); - } } } } @@ -147,11 +131,8 @@ pub fn resolve_interior<'a, 'tcx>( ) { let body = fcx.tcx.hir().body(body_id); - 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, @@ -243,7 +224,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, false); + self.record(ty, Some(scope), None, pat.span); } } @@ -284,7 +265,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, false); + self.record(adjusted_ty, scope, Some(expr), expr.span); } // Also record the unadjusted type (which is the only type if @@ -312,17 +293,9 @@ 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, false); + self.record(ty, scope, Some(expr), expr.span); } 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) { - 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); - } - } } } From 382a963c17122470918e1491a733b81fb545330d Mon Sep 17 00:00:00 2001 From: csmoe Date: Wed, 6 May 2020 16:48:52 +0800 Subject: [PATCH 5/7] filter upvars that cause trait obligation --- .../traits/error_reporting/suggestions.rs | 254 +++++++++--------- 1 file changed, 134 insertions(+), 120 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 9525910e39c6f..d2d9c303ea19d 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -125,11 +125,8 @@ pub trait InferCtxtExt<'tcx> { fn note_obligation_cause_for_async_await( &self, err: &mut DiagnosticBuilder<'_>, - target_span: Span, - scope_span: &Option, - yield_span: Option, - expr: Option, - snippet: String, + interior: Option<(Span, Option, Option, Option, Option)>, + upvar: Option<(Ty<'tcx>, Span)>, inner_generator_body: Option<&hir::Body<'_>>, outer_generator: Option, trait_ref: ty::TraitRef<'_>, @@ -137,7 +134,6 @@ pub trait InferCtxtExt<'tcx> { tables: &ty::TypeckTables<'_>, obligation: &PredicateObligation<'tcx>, next_code: Option<&ObligationCauseCode<'tcx>>, - from_awaited_ty: Option, ); fn note_obligation_cause_code( @@ -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 @@ -1173,6 +1168,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }; let mut generator = None; let mut outer_generator = None; + let mut generator_substs = None; let mut next_code = Some(&obligation.cause.code); while let Some(code) = next_code { debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code); @@ -1188,8 +1184,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); match ty.kind { - ty::Generator(did, ..) => { + ty::Generator(did, substs, ..) => { generator = generator.or(Some(did)); + generator_substs = generator_substs.or(Some(substs)); outer_generator = Some(did); } ty::GeneratorWitness(..) => {} @@ -1212,12 +1209,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { target_ty={:?}", generator, trait_ref, target_ty ); - let (generator_did, trait_ref, target_ty) = match (generator, trait_ref, target_ty) { - (Some(generator_did), Some(trait_ref), Some(target_ty)) => { - (generator_did, trait_ref, target_ty) - } - _ => return false, - }; + let (generator_did, _generator_substs, trait_ref, target_ty) = + match (generator, generator_substs, trait_ref, target_ty) { + (Some(generator_did), Some(generator_substs), Some(trait_ref), Some(target_ty)) => { + (generator_did, generator_substs, trait_ref, target_ty) + } + _ => return false, + }; let span = self.tcx.def_span(generator_did); @@ -1285,7 +1283,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)) @@ -1306,31 +1304,29 @@ 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, @@ -1338,7 +1334,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { tables, obligation, next_code, - from_awaited_ty, ); true } else { @@ -1351,11 +1346,8 @@ 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, - yield_span: Option, - expr: Option, - snippet: String, + interior: Option<(Span, Option, Option, Option, Option)>, + upvar: Option<(Ty<'tcx>, Span)>, inner_generator_body: Option<&hir::Body<'_>>, outer_generator: Option, trait_ref: ty::TraitRef<'_>, @@ -1363,7 +1355,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { tables: &ty::TypeckTables<'_>, obligation: &PredicateObligation<'tcx>, next_code: Option<&ObligationCauseCode<'tcx>>, - from_awaited_ty: Option, ) { let source_map = self.tcx.sess.source_map(); @@ -1424,99 +1415,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); - - span.push_span_label( - await_span, - format!("await occurs here on type `{}`, which {}", target_ty, trait_explanation), - ); + 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); - 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 - ); - - 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), - ); - - span.push_span_label( - target_span, - format!("has type `{}` which {}", target_ty, trait_explanation), + await_span, + format!( + "await occurs here on type `{}`, which {}", + target_ty, trait_explanation + ), ); - // 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 + "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 + ); - 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 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 (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", + 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( + upvar_span, + format!("has type `{}` which {}", target_ty, trait_explanation), + ); + } + 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 From d2e5a8e2e966df119021e3db6f6d4c2e189865cd Mon Sep 17 00:00:00 2001 From: csmoe Date: Wed, 6 May 2020 16:49:30 +0800 Subject: [PATCH 6/7] bless issue-70818 test case --- .../traits/error_reporting/suggestions.rs | 17 ++++++-------- src/test/ui/async-await/issue-70818.rs | 8 ++++--- src/test/ui/async-await/issue-70818.stderr | 23 +++++++++++++++++++ 3 files changed, 35 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/async-await/issue-70818.stderr diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index d2d9c303ea19d..35f3f5b0bbaf7 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1168,7 +1168,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }; let mut generator = None; let mut outer_generator = None; - let mut generator_substs = None; let mut next_code = Some(&obligation.cause.code); while let Some(code) = next_code { debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code); @@ -1184,9 +1183,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); match ty.kind { - ty::Generator(did, substs, ..) => { + ty::Generator(did, ..) => { generator = generator.or(Some(did)); - generator_substs = generator_substs.or(Some(substs)); outer_generator = Some(did); } ty::GeneratorWitness(..) => {} @@ -1209,13 +1207,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { target_ty={:?}", generator, trait_ref, target_ty ); - let (generator_did, _generator_substs, trait_ref, target_ty) = - match (generator, generator_substs, trait_ref, target_ty) { - (Some(generator_did), Some(generator_substs), Some(trait_ref), Some(target_ty)) => { - (generator_did, generator_substs, trait_ref, target_ty) - } - _ => return false, - }; + let (generator_did, trait_ref, target_ty) = match (generator, trait_ref, target_ty) { + (Some(generator_did), Some(trait_ref), Some(target_ty)) => { + (generator_did, trait_ref, target_ty) + } + _ => return false, + }; let span = self.tcx.def_span(generator_did); diff --git a/src/test/ui/async-await/issue-70818.rs b/src/test/ui/async-await/issue-70818.rs index 9bbaacd2f11b9..0609e4fc08170 100644 --- a/src/test/ui/async-await/issue-70818.rs +++ b/src/test/ui/async-await/issue-70818.rs @@ -1,7 +1,9 @@ -// edition 2018 +// edition:2018 -fn foo(ty: T) -> impl std::future::Future + Send { //~ Error `T` cannot be sent between threads safely - async { ty } +use std::future::Future; +fn foo(ty: T, ty1: U) -> impl Future + Send { +//~^ Error future cannot be sent between threads safely + async { (ty, ty1) } } fn main() {} diff --git a/src/test/ui/async-await/issue-70818.stderr b/src/test/ui/async-await/issue-70818.stderr new file mode 100644 index 0000000000000..97f5bde69b0c0 --- /dev/null +++ b/src/test/ui/async-await/issue-70818.stderr @@ -0,0 +1,23 @@ +error: future cannot be sent between threads safely + --> $DIR/issue-70818.rs:4:38 + | +LL | fn foo(ty: T, ty1: U) -> impl Future + 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(ty: T, ty1: U) -> impl Future + Send { + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From c9e146515ba391410069a6fe3e55a4a1935bc5ae Mon Sep 17 00:00:00 2001 From: csmoe Date: Thu, 7 May 2020 22:56:17 +0800 Subject: [PATCH 7/7] checking on either interior or upvar --- src/librustc_middle/ty/context.rs | 2 +- .../traits/error_reporting/suggestions.rs | 251 +++++++++--------- .../check/generator_interior.rs | 3 +- src/test/ui/async-await/issue-70818.stderr | 2 +- 4 files changed, 135 insertions(+), 123 deletions(-) diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index cc89e59096d89..e43eb01ad96f7 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -305,7 +305,7 @@ pub struct GeneratorInteriorTypeCause<'tcx> { /// Span of the scope of the captured binding. pub scope_span: Option, /// Span of `.await` or `yield` expression. - pub yield_span: Option, + pub yield_span: Span, /// Expr which the type evaluated from. pub expr: Option, } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 35f3f5b0bbaf7..de6131f065866 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -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( @@ -125,8 +133,8 @@ pub trait InferCtxtExt<'tcx> { fn note_obligation_cause_for_async_await( &self, err: &mut DiagnosticBuilder<'_>, - interior: Option<(Span, Option, Option, Option, Option)>, - upvar: Option<(Ty<'tcx>, Span)>, + interior_or_upvar_span: GeneratorInteriorOrUpvar, + interior_extra_info: Option<(Option, Span, Option, Option)>, inner_generator_body: Option<&hir::Body<'_>>, outer_generator: Option, trait_ref: ty::TraitRef<'_>, @@ -1280,7 +1288,23 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ); eq }; - let interior = 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)) @@ -1301,29 +1325,21 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .map(|expr| expr.span); let ty::GeneratorInteriorTypeCause { span, scope_span, yield_span, expr, .. } = cause; - (*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 - }; + 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: interior={:?} \ - generator_interior_types={:?} upvar: {:?}", - interior, tables.generator_interior_types, upvar + "maybe_note_obligation_cause_for_async_await: interior_or_upvar={:?} \ + generator_interior_types={:?}", + interior_or_upvar_span, tables.generator_interior_types ); - if interior.is_some() || upvar.is_some() { + if let Some(interior_or_upvar_span) = interior_or_upvar_span { self.note_obligation_cause_for_async_await( err, - interior, - upvar, + interior_or_upvar_span, + interior_extra_info, generator_body, outer_generator, trait_ref, @@ -1343,8 +1359,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { fn note_obligation_cause_for_async_await( &self, err: &mut DiagnosticBuilder<'_>, - interior: Option<(Span, Option, Option, Option, Option)>, - upvar: Option<(Ty<'tcx>, Span)>, + interior_or_upvar_span: GeneratorInteriorOrUpvar, + interior_extra_info: Option<(Option, Span, Option, Option)>, inner_generator_body: Option<&hir::Body<'_>>, outer_generator: Option, trait_ref: ty::TraitRef<'_>, @@ -1412,121 +1428,118 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { format!("does not implement `{}`", trait_ref.print_only_trait_path()) }; - 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); - + let mut explain_yield = |interior_span: Span, + yield_span: Span, + scope_span: Option| { + let mut span = MultiSpan::from_span(yield_span); + if let Ok(snippet) = source_map.span_to_snippet(interior_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 + 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), + ); - 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) { + err.span_note( + span, + &format!( + "{} {} as this value is used across {}", + future_or_generator, trait_explanation, an_await_or_yield + ), + ); + }; + 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( - yield_span, + await_span, format!( - "{} occurs here, with `{}` maybe used later", - await_or_yield, snippet + "await occurs here on type `{}`, which {}", + target_ty, trait_explanation ), ); - // 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!( + "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); } - span.push_span_label( - 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 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), - ); - } - 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 { + 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 outer value {}", trait_explanation)); + err.span_note(span, &format!("captured value {}", trait_explanation)); } } diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 7a5f74384f8c3..ce376a08ea604 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -96,7 +96,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { span: source_span, ty: &ty, scope_span, - yield_span: Some(yield_data.span), + yield_span: yield_data.span, expr: expr.map(|e| e.hir_id), }) .or_insert(entries); @@ -130,7 +130,6 @@ 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(), diff --git a/src/test/ui/async-await/issue-70818.stderr b/src/test/ui/async-await/issue-70818.stderr index 97f5bde69b0c0..5fb772fa10acb 100644 --- a/src/test/ui/async-await/issue-70818.stderr +++ b/src/test/ui/async-await/issue-70818.stderr @@ -8,7 +8,7 @@ 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` +note: captured value is not `Send` --> $DIR/issue-70818.rs:6:18 | LL | async { (ty, ty1) }