From 148c1bca0f0eb8da23a50cd876cb86cb060a8375 Mon Sep 17 00:00:00 2001 From: csmoe Date: Tue, 14 Jan 2020 21:21:14 +0800 Subject: [PATCH 1/4] record generoator interior exprs in typecktable --- src/librustc/traits/error_reporting.rs | 33 +++++++++++++++++++++----- src/librustc/ty/adjustment.rs | 9 +++++++ src/librustc/ty/context.rs | 5 ++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 0c9a73d78a5eb..66737372bda37 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2456,7 +2456,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let target_span = tables .generator_interior_types .iter() - .find(|ty::GeneratorInteriorTypeCause { ty, .. }| { + .zip(tables.generator_interior_exprs.iter()) + .find(|(ty::GeneratorInteriorTypeCause { ty, .. }, _)| { // Careful: the regions for types that appear in the // generator interior are not generally known, so we // want to erase them when comparing (and anyway, @@ -2479,19 +2480,21 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ); eq }) - .map(|ty::GeneratorInteriorTypeCause { span, scope_span, .. }| { - (span, source_map.span_to_snippet(*span), scope_span) + .map(|(ty::GeneratorInteriorTypeCause { span, scope_span, .. }, expr)| { + (span, source_map.span_to_snippet(*span), scope_span, expr) }); + debug!( "maybe_note_obligation_cause_for_async_await: target_ty={:?} \ generator_interior_types={:?} target_span={:?}", target_ty, tables.generator_interior_types, target_span ); - if let Some((target_span, Ok(snippet), scope_span)) = target_span { + if let Some((target_span, Ok(snippet), scope_span, expr)) = target_span { self.note_obligation_cause_for_async_await( err, *target_span, scope_span, + *expr, snippet, generator_did, last_generator, @@ -2514,6 +2517,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, target_span: Span, scope_span: &Option, + expr: Option, snippet: String, first_generator: DefId, last_generator: Option, @@ -2549,6 +2553,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // not implemented. let is_send = self.tcx.is_diagnostic_item(sym::send_trait, trait_ref.def_id); let is_sync = self.tcx.is_diagnostic_item(sym::sync_trait, trait_ref.def_id); + let hir = self.tcx.hir(); let trait_explanation = if is_send || is_sync { let (trait_name, trait_verb) = if is_send { ("`Send`", "sent") } else { ("`Sync`", "shared") }; @@ -2564,8 +2569,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let message = if let Some(name) = last_generator .and_then(|generator_did| self.tcx.parent(generator_did)) - .and_then(|parent_did| self.tcx.hir().as_local_hir_id(parent_did)) - .and_then(|parent_hir_id| self.tcx.hir().opt_name(parent_hir_id)) + .and_then(|parent_did| hir.as_local_hir_id(parent_did)) + .and_then(|parent_hir_id| hir.opt_name(parent_hir_id)) { format!("future returned by `{}` is not {}", name, trait_name) } else { @@ -2588,6 +2593,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); + if let Some(expr_id) = expr { + let expr = hir.expect_expr(expr_id); + let is_ref = tables.expr_adjustments(expr).iter().any(|adj| adj.is_region_borrow()); + let parent = hir.get_parent_node(expr_id); + if let Some(hir::Node::Expr(e)) = hir.find(parent) { + let method_span = hir.span(parent); + if tables.is_method_call(e) && is_ref { + err.span_help( + method_span, + "consider moving this method call into a `let` \ + binding to create a shorter lived borrow" + ); + } + } + } + span.push_span_label(target_span, format!("has type `{}`", target_ty)); // If available, use the scope span to annotate the drop location. diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index ebefb03b813f2..6531289bd7050 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -81,6 +81,15 @@ pub struct Adjustment<'tcx> { pub target: Ty<'tcx>, } +impl Adjustment<'tcx> { + pub fn is_region_borrow(&self) -> bool { + match self.kind { + Adjust::Borrow(AutoBorrow::Ref(..)) => true, + _ => false + } + } +} + #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub enum Adjust<'tcx> { /// Go from ! to any type. diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 6b98fddb22ef9..4a5fced7a89e5 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -439,6 +439,8 @@ pub struct TypeckTables<'tcx> { /// Stores the type, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). pub generator_interior_types: Vec>, + + pub generator_interior_exprs: Vec>, } impl<'tcx> TypeckTables<'tcx> { @@ -465,6 +467,7 @@ impl<'tcx> TypeckTables<'tcx> { concrete_opaque_types: Default::default(), upvar_list: Default::default(), generator_interior_types: Default::default(), + generator_interior_exprs: Default::default(), } } @@ -728,6 +731,7 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { ref concrete_opaque_types, ref upvar_list, ref generator_interior_types, + ref generator_interior_exprs, } = *self; hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { @@ -766,6 +770,7 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { concrete_opaque_types.hash_stable(hcx, hasher); upvar_list.hash_stable(hcx, hasher); generator_interior_types.hash_stable(hcx, hasher); + generator_interior_exprs.hash_stable(hcx, hasher); }) } } From b97560116785f2dbb488e66ab34aeee981ec2912 Mon Sep 17 00:00:00 2001 From: csmoe Date: Tue, 14 Jan 2020 21:22:19 +0800 Subject: [PATCH 2/4] suggest to limit lifetime of temporary borrow with let --- src/librustc/traits/error_reporting.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 66737372bda37..13eb47a041fa6 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2578,7 +2578,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; span.push_span_label(original_span, message); - err.set_span(span); + err.set_span(span.clone()); format!("is not {}", trait_name) } else { From 5ad8b9e3948fa05df0b9c2a6c71146c3af10cbc8 Mon Sep 17 00:00:00 2001 From: csmoe Date: Tue, 14 Jan 2020 21:22:43 +0800 Subject: [PATCH 3/4] update async-await send/sync test --- src/librustc/traits/error_reporting.rs | 38 +++++++++---------- src/librustc/ty/adjustment.rs | 2 +- src/librustc/ty/context.rs | 9 +---- .../check/generator_interior.rs | 6 ++- .../issue-64130-4-async-move.stderr | 5 +++ 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 13eb47a041fa6..aaad0be9a048c 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2456,7 +2456,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let target_span = tables .generator_interior_types .iter() - .zip(tables.generator_interior_exprs.iter()) .find(|(ty::GeneratorInteriorTypeCause { ty, .. }, _)| { // Careful: the regions for types that appear in the // generator interior are not generally known, so we @@ -2578,7 +2577,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; span.push_span_label(original_span, message); - err.set_span(span.clone()); + err.set_span(span); format!("is not {}", trait_name) } else { @@ -2586,29 +2585,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; // Look at the last interior type to get a span for the `.await`. - let await_span = tables.generator_interior_types.iter().map(|i| i.span).last().unwrap(); + let await_span = + tables.generator_interior_types.iter().map(|(i, _)| i.span).last().unwrap(); let mut span = MultiSpan::from_span(await_span); span.push_span_label( await_span, format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); - if let Some(expr_id) = expr { - let expr = hir.expect_expr(expr_id); - let is_ref = tables.expr_adjustments(expr).iter().any(|adj| adj.is_region_borrow()); - let parent = hir.get_parent_node(expr_id); - if let Some(hir::Node::Expr(e)) = hir.find(parent) { - let method_span = hir.span(parent); - if tables.is_method_call(e) && is_ref { - err.span_help( - method_span, - "consider moving this method call into a `let` \ - binding to create a shorter lived borrow" - ); - } - } - } - span.push_span_label(target_span, format!("has type `{}`", target_ty)); // If available, use the scope span to annotate the drop location. @@ -2627,6 +2611,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ), ); + if let Some(expr_id) = expr { + let expr = hir.expect_expr(expr_id); + let is_ref = tables.expr_adjustments(expr).iter().any(|adj| adj.is_region_borrow()); + let parent = hir.get_parent_node(expr_id); + if let Some(hir::Node::Expr(e)) = hir.find(parent) { + let method_span = hir.span(parent); + if tables.is_method_call(e) && is_ref { + err.span_help( + method_span, + "consider moving this method call into a `let` \ + binding to create a shorter lived borrow", + ); + } + } + } + // Add a note for the item obligation that remains - normally a note pointing to the // bound that introduced the obligation (e.g. `T: Send`). debug!("note_obligation_cause_for_async_await: next_code={:?}", next_code); diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index 6531289bd7050..db034d1618cea 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -85,7 +85,7 @@ impl Adjustment<'tcx> { pub fn is_region_borrow(&self) -> bool { match self.kind { Adjust::Borrow(AutoBorrow::Ref(..)) => true, - _ => false + _ => false, } } } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 4a5fced7a89e5..d64e049c47b54 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -436,11 +436,9 @@ pub struct TypeckTables<'tcx> { /// entire variable. pub upvar_list: ty::UpvarListMap, - /// Stores the type, span and optional scope span of all types + /// Stores the type, expression, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). - pub generator_interior_types: Vec>, - - pub generator_interior_exprs: Vec>, + pub generator_interior_types: Vec<(GeneratorInteriorTypeCause<'tcx>, Option)>, } impl<'tcx> TypeckTables<'tcx> { @@ -467,7 +465,6 @@ impl<'tcx> TypeckTables<'tcx> { concrete_opaque_types: Default::default(), upvar_list: Default::default(), generator_interior_types: Default::default(), - generator_interior_exprs: Default::default(), } } @@ -731,7 +728,6 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { ref concrete_opaque_types, ref upvar_list, ref generator_interior_types, - ref generator_interior_exprs, } = *self; hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { @@ -770,7 +766,6 @@ impl<'a, 'tcx> HashStable> for TypeckTables<'tcx> { concrete_opaque_types.hash_stable(hcx, hasher); upvar_list.hash_stable(hcx, hasher); generator_interior_types.hash_stable(hcx, hasher); - generator_interior_exprs.hash_stable(hcx, hasher); }) } } diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 9d8805f225d7e..7185de76fcbb6 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -18,6 +18,7 @@ use rustc_span::Span; struct InteriorVisitor<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, types: FxHashMap, usize>, + exprs: Vec>, region_scope_tree: &'tcx region::ScopeTree, expr_count: usize, kind: hir::GeneratorKind, @@ -99,6 +100,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { scope_span, }) .or_insert(entries); + self.exprs.push(expr.map(|e| e.hir_id)); } } else { debug!( @@ -136,6 +138,7 @@ pub fn resolve_interior<'a, 'tcx>( expr_count: 0, kind, prev_unresolved_span: None, + exprs: vec![], }; intravisit::walk_body(&mut visitor, body); @@ -170,7 +173,8 @@ pub fn resolve_interior<'a, 'tcx>( }); // Store the generator types and spans into the tables for this generator. - let interior_types = types.iter().map(|t| t.0.clone()).collect::>(); + let interior_types = + types.iter().zip(visitor.exprs).map(|(t, e)| (t.0.clone(), e)).collect::>(); visitor.fcx.inh.tables.borrow_mut().generator_interior_types = interior_types; // Extract type components diff --git a/src/test/ui/async-await/issue-64130-4-async-move.stderr b/src/test/ui/async-await/issue-64130-4-async-move.stderr index ddbb469b99c24..77d0885c38d58 100644 --- a/src/test/ui/async-await/issue-64130-4-async-move.stderr +++ b/src/test/ui/async-await/issue-64130-4-async-move.stderr @@ -16,6 +16,11 @@ LL | let _x = get().await; ... LL | } | - `client` is later dropped here +help: consider moving this method call into a `let` binding to create a shorter lived borrow + --> $DIR/issue-64130-4-async-move.rs:19:15 + | +LL | match client.status() { + | ^^^^^^^^^^^^^^^ = note: the return type of a function must have a statically known size error: aborting due to previous error From 4eb47ded54610300c54291aee74d5585a711e75b Mon Sep 17 00:00:00 2001 From: csmoe Date: Wed, 15 Jan 2020 15:13:51 +0800 Subject: [PATCH 4/4] wrap expr id into GeneratorInteriorTypeCause --- src/librustc/traits/error_reporting.rs | 7 +++---- src/librustc/ty/context.rs | 7 ++++--- .../check/generator_interior.rs | 21 ++++++++++++------- src/test/ui/generator/not-send-sync.stderr | 2 +- .../recursive-impl-trait-type-indirect.stderr | 4 ++-- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index aaad0be9a048c..c1df1149bbdfc 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -2456,7 +2456,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let target_span = tables .generator_interior_types .iter() - .find(|(ty::GeneratorInteriorTypeCause { ty, .. }, _)| { + .find(|ty::GeneratorInteriorTypeCause { ty, .. }| { // Careful: the regions for types that appear in the // generator interior are not generally known, so we // want to erase them when comparing (and anyway, @@ -2479,7 +2479,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ); eq }) - .map(|(ty::GeneratorInteriorTypeCause { span, scope_span, .. }, expr)| { + .map(|ty::GeneratorInteriorTypeCause { span, scope_span, expr, .. }| { (span, source_map.span_to_snippet(*span), scope_span, expr) }); @@ -2585,8 +2585,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; // Look at the last interior type to get a span for the `.await`. - let await_span = - tables.generator_interior_types.iter().map(|(i, _)| i.span).last().unwrap(); + let await_span = tables.generator_interior_types.iter().map(|t| t.span).last().unwrap(); let mut span = MultiSpan::from_span(await_span); span.push_span_label( await_span, diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index d64e049c47b54..ef776c88a8f7c 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -315,8 +315,7 @@ pub struct ResolvedOpaqueTy<'tcx> { /// /// Here, we would store the type `T`, the span of the value `x`, and the "scope-span" for /// the scope that contains `x`. -#[derive(RustcEncodable, RustcDecodable, Clone, Debug, Eq, Hash, PartialEq)] -#[derive(HashStable, TypeFoldable)] +#[derive(RustcEncodable, RustcDecodable, Clone, Debug, Eq, Hash, PartialEq, HashStable)] pub struct GeneratorInteriorTypeCause<'tcx> { /// Type of the captured binding. pub ty: Ty<'tcx>, @@ -324,6 +323,8 @@ pub struct GeneratorInteriorTypeCause<'tcx> { pub span: Span, /// Span of the scope of the captured binding. pub scope_span: Option, + /// Expr which the type evaluated from. + pub expr: Option, } #[derive(RustcEncodable, RustcDecodable, Debug)] @@ -438,7 +439,7 @@ pub struct TypeckTables<'tcx> { /// Stores the type, expression, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). - pub generator_interior_types: Vec<(GeneratorInteriorTypeCause<'tcx>, Option)>, + pub generator_interior_types: Vec>, } impl<'tcx> TypeckTables<'tcx> { diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 7185de76fcbb6..fc02d17a50f37 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -18,7 +18,6 @@ use rustc_span::Span; struct InteriorVisitor<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, types: FxHashMap, usize>, - exprs: Vec>, region_scope_tree: &'tcx region::ScopeTree, expr_count: usize, kind: hir::GeneratorKind, @@ -98,9 +97,9 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { span: source_span, ty: &ty, scope_span, + expr: expr.map(|e| e.hir_id), }) .or_insert(entries); - self.exprs.push(expr.map(|e| e.hir_id)); } } else { debug!( @@ -138,7 +137,6 @@ pub fn resolve_interior<'a, 'tcx>( expr_count: 0, kind, prev_unresolved_span: None, - exprs: vec![], }; intravisit::walk_body(&mut visitor, body); @@ -167,18 +165,25 @@ pub fn resolve_interior<'a, 'tcx>( // which means that none of the regions inside relate to any other, even if // typeck had previously found constraints that would cause them to be related. let mut counter = 0; - let types = fcx.tcx.fold_regions(&types, &mut false, |_, current_depth| { + let fold_types: Vec<_> = types.iter().map(|(t, _)| t.ty).collect(); + let folded_types = fcx.tcx.fold_regions(&fold_types, &mut false, |_, current_depth| { counter += 1; fcx.tcx.mk_region(ty::ReLateBound(current_depth, ty::BrAnon(counter))) }); // Store the generator types and spans into the tables for this generator. - let interior_types = - types.iter().zip(visitor.exprs).map(|(t, e)| (t.0.clone(), e)).collect::>(); - visitor.fcx.inh.tables.borrow_mut().generator_interior_types = interior_types; + let types = types + .into_iter() + .zip(&folded_types) + .map(|((mut interior_cause, _), ty)| { + interior_cause.ty = ty; + interior_cause + }) + .collect(); + visitor.fcx.inh.tables.borrow_mut().generator_interior_types = types; // Extract type components - let type_list = fcx.tcx.mk_type_list(types.into_iter().map(|t| (t.0).ty)); + let type_list = fcx.tcx.mk_type_list(folded_types.iter()); let witness = fcx.tcx.mk_generator_witness(ty::Binder::bind(type_list)); diff --git a/src/test/ui/generator/not-send-sync.stderr b/src/test/ui/generator/not-send-sync.stderr index 0ac1d189b79b0..18d9012b3acf6 100644 --- a/src/test/ui/generator/not-send-sync.stderr +++ b/src/test/ui/generator/not-send-sync.stderr @@ -20,7 +20,7 @@ LL | fn assert_sync(_: T) {} LL | assert_sync(|| { | ^^^^^^^^^^^ future returned by `main` is not `Sync` | - = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell` + = help: within `[generator@$DIR/not-send-sync.rs:9:17: 13:6 {std::cell::Cell, (), ()}]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell` note: future is not `Sync` as this value is used across an yield --> $DIR/not-send-sync.rs:12:9 | diff --git a/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr b/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr index b7ba0d6ab177c..15a028f60ae11 100644 --- a/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr +++ b/src/test/ui/impl-trait/recursive-impl-trait-type-indirect.stderr @@ -76,7 +76,7 @@ error[E0720]: opaque type expands to a recursive type LL | fn generator_capture() -> impl Sized { | ^^^^^^^^^^ expands to a recursive type | - = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:50:5: 50:26 x:impl Sized {()}]` + = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:50:5: 50:26 x:impl Sized {(), ()}]` error[E0720]: opaque type expands to a recursive type --> $DIR/recursive-impl-trait-type-indirect.rs:53:26 @@ -92,7 +92,7 @@ error[E0720]: opaque type expands to a recursive type LL | fn generator_hold() -> impl Sized { | ^^^^^^^^^^ expands to a recursive type | - = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:58:5: 62:6 {impl Sized, ()}]` + = note: expanded type is `[generator@$DIR/recursive-impl-trait-type-indirect.rs:58:5: 62:6 {impl Sized, (), ()}]` error[E0720]: opaque type expands to a recursive type --> $DIR/recursive-impl-trait-type-indirect.rs:69:26