From 382a963c17122470918e1491a733b81fb545330d Mon Sep 17 00:00:00 2001 From: csmoe Date: Wed, 6 May 2020 16:48:52 +0800 Subject: [PATCH] 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 9525910e39c6..d2d9c303ea19 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), - ); - - 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), - ); + 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( - 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); + 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), + ); - 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", + 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