From e07f63e79a618a0c5e53a71d053da0fa0dbc6970 Mon Sep 17 00:00:00 2001 From: csmoe Date: Thu, 30 Apr 2020 12:15:59 +0800 Subject: [PATCH 01/15] 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 02/15] 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 03/15] 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 04/15] 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 05/15] 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 06/15] 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 07/15] 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) } From afa75d6c0afc0dcb691ef23e4f578c10ce52560d Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Tue, 12 May 2020 22:18:55 +0200 Subject: [PATCH 08/15] exhaustively match during structural match checking --- .../hair/pattern/const_to_pat.rs | 5 ++- .../traits/structural_match.rs | 35 +++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/const_to_pat.rs b/src/librustc_mir_build/hair/pattern/const_to_pat.rs index 854f8eeaf3441..1c36a65dd0764 100644 --- a/src/librustc_mir_build/hair/pattern/const_to_pat.rs +++ b/src/librustc_mir_build/hair/pattern/const_to_pat.rs @@ -124,7 +124,10 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { "trait objects cannot be used in patterns".to_string() } traits::NonStructuralMatchTy::Param => { - bug!("use of constant whose type is a parameter inside a pattern") + bug!("use of a constant whose type is a parameter inside a pattern") + } + traits::NonStructuralMatchTy::Foreign => { + bug!("use of a value of a foreign type inside a pattern") } }; diff --git a/src/librustc_trait_selection/traits/structural_match.rs b/src/librustc_trait_selection/traits/structural_match.rs index 8007290f35d85..18279310d7304 100644 --- a/src/librustc_trait_selection/traits/structural_match.rs +++ b/src/librustc_trait_selection/traits/structural_match.rs @@ -12,6 +12,7 @@ pub enum NonStructuralMatchTy<'tcx> { Adt(&'tcx AdtDef), Param, Dynamic, + Foreign, } /// This method traverses the structure of `ty`, trying to find an @@ -142,6 +143,10 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { self.found = Some(NonStructuralMatchTy::Dynamic); return true; // Stop visiting. } + ty::Foreign(_) => { + self.found = Some(NonStructuralMatchTy::Foreign); + return true; // Stop visiting + } ty::RawPtr(..) => { // structural-match ignores substructure of // `*const _`/`*mut _`, so skip `super_visit_with`. @@ -162,7 +167,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { return false; } ty::FnDef(..) | ty::FnPtr(..) => { - // types of formals and return in `fn(_) -> _` are also irrelevant; + // Types of formals and return in `fn(_) -> _` are also irrelevant; // so we do not recur into them via `super_visit_with` // // (But still tell caller to continue search.) @@ -175,7 +180,33 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { // for empty array. return false; } - _ => { + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Str + | ty::Never + | ty::Error => { + // These primitive types are always structural match. + // + // `Never` is kind of special here, but as it is not inhabitable, this should be fine. + return false; + } + + ty::Array(..) + | ty::Slice(_) + | ty::Ref(..) + | ty::Closure(..) + | ty::Generator(..) + | ty::GeneratorWitness(..) + | ty::Tuple(..) + | ty::Projection(..) + | ty::UnnormalizedProjection(..) + | ty::Opaque(..) + | ty::Bound(..) + | ty::Placeholder(_) + | ty::Infer(_) => { ty.super_visit_with(self); return false; } From f37d606dd687ec6e4c6bee0b389d71b71f0840ca Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Tue, 12 May 2020 23:35:29 +0200 Subject: [PATCH 09/15] note for `ty::Error`. --- .../traits/structural_match.rs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/librustc_trait_selection/traits/structural_match.rs b/src/librustc_trait_selection/traits/structural_match.rs index 18279310d7304..b7514aa02971d 100644 --- a/src/librustc_trait_selection/traits/structural_match.rs +++ b/src/librustc_trait_selection/traits/structural_match.rs @@ -186,8 +186,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { | ty::Uint(_) | ty::Float(_) | ty::Str - | ty::Never - | ty::Error => { + | ty::Never => { // These primitive types are always structural match. // // `Never` is kind of special here, but as it is not inhabitable, this should be fine. @@ -199,17 +198,25 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { | ty::Ref(..) | ty::Closure(..) | ty::Generator(..) - | ty::GeneratorWitness(..) | ty::Tuple(..) | ty::Projection(..) - | ty::UnnormalizedProjection(..) | ty::Opaque(..) - | ty::Bound(..) - | ty::Placeholder(_) - | ty::Infer(_) => { + | ty::GeneratorWitness(..) => { ty.super_visit_with(self); return false; } + | ty::Infer(_) + | ty::Placeholder(_) + | ty::UnnormalizedProjection(..) + | ty::Bound(..) => { + bug!("unexpected type during structural-match checking: {:?}", ty); + } + ty::Error => { + self.tcx().delay_span_bug(self.span, "ty::Error in structural-match check"); + // We still want to check other types after encountering an error, + // as this may still emit relevant errors. + return false; + } }; if !self.seen.insert(adt_def.did) { From 2a69df207e16c0b3425b06973b3edd52cff8a25f Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Wed, 13 May 2020 00:40:28 +0200 Subject: [PATCH 10/15] Be more conservative concerning `structural_match` --- .../hair/pattern/const_to_pat.rs | 9 +++++ .../traits/structural_match.rs | 36 ++++++++++--------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/const_to_pat.rs b/src/librustc_mir_build/hair/pattern/const_to_pat.rs index 1c36a65dd0764..b3cab41017251 100644 --- a/src/librustc_mir_build/hair/pattern/const_to_pat.rs +++ b/src/librustc_mir_build/hair/pattern/const_to_pat.rs @@ -123,9 +123,18 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { traits::NonStructuralMatchTy::Dynamic => { "trait objects cannot be used in patterns".to_string() } + traits::NonStructuralMatchTy::Opaque => { + "opaque types cannot be used in patterns".to_string() + } + traits::NonStructuralMatchTy::Generator => { + "generators cannot be used in patterns".to_string() + } traits::NonStructuralMatchTy::Param => { bug!("use of a constant whose type is a parameter inside a pattern") } + traits::NonStructuralMatchTy::Projection => { + bug!("use of a constant whose type is a projection inside a pattern") + } traits::NonStructuralMatchTy::Foreign => { bug!("use of a value of a foreign type inside a pattern") } diff --git a/src/librustc_trait_selection/traits/structural_match.rs b/src/librustc_trait_selection/traits/structural_match.rs index b7514aa02971d..ae66bbf26902a 100644 --- a/src/librustc_trait_selection/traits/structural_match.rs +++ b/src/librustc_trait_selection/traits/structural_match.rs @@ -13,6 +13,9 @@ pub enum NonStructuralMatchTy<'tcx> { Param, Dynamic, Foreign, + Opaque, + Generator, + Projection, } /// This method traverses the structure of `ty`, trying to find an @@ -147,6 +150,18 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { self.found = Some(NonStructuralMatchTy::Foreign); return true; // Stop visiting } + ty::Opaque(..) => { + self.found = Some(NonStructuralMatchTy::Opaque); + return true; + } + ty::Projection(..) => { + self.found = Some(NonStructuralMatchTy::Projection); + return true; + } + ty::Generator(..) | ty::GeneratorWitness(..) => { + self.found = Some(NonStructuralMatchTy::Generator); + return true; + } ty::RawPtr(..) => { // structural-match ignores substructure of // `*const _`/`*mut _`, so skip `super_visit_with`. @@ -180,31 +195,18 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { // for empty array. return false; } - ty::Bool - | ty::Char - | ty::Int(_) - | ty::Uint(_) - | ty::Float(_) - | ty::Str - | ty::Never => { + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str | ty::Never => { // These primitive types are always structural match. // // `Never` is kind of special here, but as it is not inhabitable, this should be fine. return false; } - ty::Array(..) - | ty::Slice(_) - | ty::Ref(..) - | ty::Closure(..) - | ty::Generator(..) - | ty::Tuple(..) - | ty::Projection(..) - | ty::Opaque(..) - | ty::GeneratorWitness(..) => { + ty::Array(..) | ty::Slice(_) | ty::Ref(..) | ty::Tuple(..) => { ty.super_visit_with(self); return false; } + ty::Closure(..) | ty::Infer(_) | ty::Placeholder(_) | ty::UnnormalizedProjection(..) @@ -212,7 +214,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { bug!("unexpected type during structural-match checking: {:?}", ty); } ty::Error => { - self.tcx().delay_span_bug(self.span, "ty::Error in structural-match check"); + self.tcx().sess.delay_span_bug(self.span, "ty::Error in structural-match check"); // We still want to check other types after encountering an error, // as this may still emit relevant errors. return false; From 516798ea50fd9a9809026ca23b01cd7e83bb56df Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Wed, 13 May 2020 11:52:22 +0200 Subject: [PATCH 11/15] comment return sites --- .../traits/structural_match.rs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/librustc_trait_selection/traits/structural_match.rs b/src/librustc_trait_selection/traits/structural_match.rs index ae66bbf26902a..51a168322a120 100644 --- a/src/librustc_trait_selection/traits/structural_match.rs +++ b/src/librustc_trait_selection/traits/structural_match.rs @@ -148,19 +148,19 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { } ty::Foreign(_) => { self.found = Some(NonStructuralMatchTy::Foreign); - return true; // Stop visiting + return true; // Stop visiting. } ty::Opaque(..) => { self.found = Some(NonStructuralMatchTy::Opaque); - return true; + return true; // Stop visiting. } ty::Projection(..) => { self.found = Some(NonStructuralMatchTy::Projection); - return true; + return true; // Stop visiting. } ty::Generator(..) | ty::GeneratorWitness(..) => { self.found = Some(NonStructuralMatchTy::Generator); - return true; + return true; // Stop visiting. } ty::RawPtr(..) => { // structural-match ignores substructure of @@ -178,14 +178,14 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { // structural equality on `T` does not recur into the raw // pointer. Therefore, one can still use `C` in a pattern. - // (But still tell caller to continue search.) + // (But still tell the caller to continue search.) return false; } ty::FnDef(..) | ty::FnPtr(..) => { // Types of formals and return in `fn(_) -> _` are also irrelevant; // so we do not recur into them via `super_visit_with` // - // (But still tell caller to continue search.) + // (But still tell the caller to continue search.) return false; } ty::Array(_, n) @@ -193,16 +193,21 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { { // rust-lang/rust#62336: ignore type of contents // for empty array. + // + // (But still tell the caller to continue search.) return false; } ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str | ty::Never => { // These primitive types are always structural match. // // `Never` is kind of special here, but as it is not inhabitable, this should be fine. + // + // (But still tell the caller to continue search.) return false; } ty::Array(..) | ty::Slice(_) | ty::Ref(..) | ty::Tuple(..) => { + // First check all contained types and then tell the caller to continue searching. ty.super_visit_with(self); return false; } @@ -217,13 +222,15 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> { self.tcx().sess.delay_span_bug(self.span, "ty::Error in structural-match check"); // We still want to check other types after encountering an error, // as this may still emit relevant errors. + // + // So we continue searching here. return false; } }; if !self.seen.insert(adt_def.did) { debug!("Search already seen adt_def: {:?}", adt_def); - // let caller continue its search + // Let caller continue its search. return false; } From 60d9df206c0d195507a540fb57c34b1fa09dc91e Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Wed, 13 May 2020 13:14:53 +0200 Subject: [PATCH 12/15] Add tests for opaque types --- .../structural-match-no-leak.rs | 20 ++++++++++++++++++ .../structural-match-no-leak.stderr | 8 +++++++ .../type-alias-impl-trait/structural-match.rs | 21 +++++++++++++++++++ .../structural-match.stderr | 8 +++++++ 4 files changed, 57 insertions(+) create mode 100644 src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs create mode 100644 src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr create mode 100644 src/test/ui/type-alias-impl-trait/structural-match.rs create mode 100644 src/test/ui/type-alias-impl-trait/structural-match.stderr diff --git a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs new file mode 100644 index 0000000000000..479d6cd9af765 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.rs @@ -0,0 +1,20 @@ +#![feature(const_fn, type_alias_impl_trait)] + +type Bar = impl Send; + +// While i32 is structural-match, we do not want to leak this information. +// (See https://github.com/rust-lang/rust/issues/72156) +const fn leak_free() -> Bar { + 7i32 +} +const LEAK_FREE: Bar = leak_free(); + +fn leak_free_test() { + match todo!() { + LEAK_FREE => (), + //~^ opaque types cannot be used in patterns + _ => (), + } +} + +fn main() { } diff --git a/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr new file mode 100644 index 0000000000000..ae0d8e8d4239c --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/structural-match-no-leak.stderr @@ -0,0 +1,8 @@ +error: opaque types cannot be used in patterns + --> $DIR/structural-match-no-leak.rs:14:9 + | +LL | LEAK_FREE => (), + | ^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/type-alias-impl-trait/structural-match.rs b/src/test/ui/type-alias-impl-trait/structural-match.rs new file mode 100644 index 0000000000000..481448d64b1aa --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/structural-match.rs @@ -0,0 +1,21 @@ +#![feature(const_fn, type_alias_impl_trait)] + +type Foo = impl Send; + +// This is not structural-match +struct A; + +const fn value() -> Foo { + A +} +const VALUE: Foo = value(); + +fn test() { + match todo!() { + VALUE => (), + //~^ opaque types cannot be used in patterns + _ => (), + } +} + +fn main() { } diff --git a/src/test/ui/type-alias-impl-trait/structural-match.stderr b/src/test/ui/type-alias-impl-trait/structural-match.stderr new file mode 100644 index 0000000000000..ad9036a87d1d9 --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/structural-match.stderr @@ -0,0 +1,8 @@ +error: opaque types cannot be used in patterns + --> $DIR/structural-match.rs:15:9 + | +LL | VALUE => (), + | ^^^^^ + +error: aborting due to previous error + From 4af04cb5cbe19cc3658584c005180da9a38c1530 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sat, 16 May 2020 18:23:54 +0100 Subject: [PATCH 13/15] Continue lowering for unsupported async generator instead of returning an error. This way the hir is "valid" and we can remove one more call to `opt_node_id_to_hir_id` but an error is still emitted. This is another partial fix for #71104 --- src/librustc_ast_lowering/expr.rs | 1 - src/librustc_middle/ty/context.rs | 17 +++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 8da3aa633b85f..c1cfb9b5f4dc7 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -1021,7 +1021,6 @@ impl<'hir> LoweringContext<'_, 'hir> { "`async` generators are not yet supported" ) .emit(); - return hir::ExprKind::Err; } None => self.generator_kind = Some(hir::GeneratorKind::Gen), } diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index 7feb080d4b8d4..865660290dc54 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -1114,16 +1114,13 @@ impl<'tcx> TyCtxt<'tcx> { let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default(); for (k, v) in resolutions.trait_map { - // FIXME(#71104) Should really be using just `node_id_to_hir_id` but - // some `NodeId` do not seem to have a corresponding HirId. - if let Some(hir_id) = definitions.opt_node_id_to_hir_id(k) { - let map = trait_map.entry(hir_id.owner).or_default(); - let v = v - .into_iter() - .map(|tc| tc.map_import_ids(|id| definitions.node_id_to_hir_id(id))) - .collect(); - map.insert(hir_id.local_id, StableVec::new(v)); - } + let hir_id = definitions.node_id_to_hir_id(k); + let map = trait_map.entry(hir_id.owner).or_default(); + let v = v + .into_iter() + .map(|tc| tc.map_import_ids(|id| definitions.node_id_to_hir_id(id))) + .collect(); + map.insert(hir_id.local_id, StableVec::new(v)); } GlobalCtxt { From ed8478036c416694db5f51f6e43078a1c07532cf Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 16 May 2020 17:51:16 +0200 Subject: [PATCH 14/15] Fix going back in history to a search result page on firefox --- src/librustdoc/html/static/main.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/librustdoc/html/static/main.js b/src/librustdoc/html/static/main.js index a023d5a2d95f1..fffabcf79fe7c 100644 --- a/src/librustdoc/html/static/main.js +++ b/src/librustdoc/html/static/main.js @@ -2703,3 +2703,9 @@ function focusSearchBar() { function defocusSearchBar() { getSearchInput().blur(); } + +// This is required in firefox. Explanations: when going back in the history, firefox doesn't re-run +// the JS, therefore preventing rustdoc from setting a few things required to be able to reload the +// previous search results (if you navigated to a search result with the keyboard, pressed enter on +// it to navigate to that result, and then came back to this page). +window.onunload = function(){}; From 46159b36103bc36d0bac8b78062f8f8834d9ad71 Mon Sep 17 00:00:00 2001 From: Pyry Kontio Date: Wed, 20 May 2020 04:22:37 +0900 Subject: [PATCH 15/15] split_inclusive: add tracking issue number (72360) --- src/libcore/slice/mod.rs | 26 +++++++++++++------------- src/libcore/str/mod.rs | 14 +++++++------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 3386f83ec810f..9582ac33ff6b7 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -1169,7 +1169,7 @@ impl [T] { /// assert_eq!(iter.next().unwrap(), &[10, 40, 33]); /// assert!(iter.next().is_none()); /// ``` - #[unstable(feature = "split_inclusive", issue = "none")] + #[unstable(feature = "split_inclusive", issue = "72360")] #[inline] pub fn split_inclusive(&self, pred: F) -> SplitInclusive<'_, T, F> where @@ -1194,7 +1194,7 @@ impl [T] { /// } /// assert_eq!(v, [10, 40, 1, 20, 1, 1]); /// ``` - #[unstable(feature = "split_inclusive", issue = "none")] + #[unstable(feature = "split_inclusive", issue = "72360")] #[inline] pub fn split_inclusive_mut(&mut self, pred: F) -> SplitInclusiveMut<'_, T, F> where @@ -3852,7 +3852,7 @@ impl FusedIterator for Split<'_, T, P> where P: FnMut(&T) -> bool {} /// /// [`split_inclusive`]: ../../std/primitive.slice.html#method.split_inclusive /// [slices]: ../../std/primitive.slice.html -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] pub struct SplitInclusive<'a, T: 'a, P> where P: FnMut(&T) -> bool, @@ -3862,7 +3862,7 @@ where finished: bool, } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl fmt::Debug for SplitInclusive<'_, T, P> where P: FnMut(&T) -> bool, @@ -3876,7 +3876,7 @@ where } // FIXME(#26925) Remove in favor of `#[derive(Clone)]` -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl Clone for SplitInclusive<'_, T, P> where P: Clone + FnMut(&T) -> bool, @@ -3886,7 +3886,7 @@ where } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, T, P> Iterator for SplitInclusive<'a, T, P> where P: FnMut(&T) -> bool, @@ -3915,7 +3915,7 @@ where } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, T, P> DoubleEndedIterator for SplitInclusive<'a, T, P> where P: FnMut(&T) -> bool, @@ -3940,7 +3940,7 @@ where } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl FusedIterator for SplitInclusive<'_, T, P> where P: FnMut(&T) -> bool {} /// An iterator over the mutable subslices of the vector which are separated @@ -4065,7 +4065,7 @@ impl FusedIterator for SplitMut<'_, T, P> where P: FnMut(&T) -> bool {} /// /// [`split_inclusive_mut`]: ../../std/primitive.slice.html#method.split_inclusive_mut /// [slices]: ../../std/primitive.slice.html -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] pub struct SplitInclusiveMut<'a, T: 'a, P> where P: FnMut(&T) -> bool, @@ -4075,7 +4075,7 @@ where finished: bool, } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl fmt::Debug for SplitInclusiveMut<'_, T, P> where P: FnMut(&T) -> bool, @@ -4088,7 +4088,7 @@ where } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, T, P> Iterator for SplitInclusiveMut<'a, T, P> where P: FnMut(&T) -> bool, @@ -4128,7 +4128,7 @@ where } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, T, P> DoubleEndedIterator for SplitInclusiveMut<'a, T, P> where P: FnMut(&T) -> bool, @@ -4162,7 +4162,7 @@ where } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl FusedIterator for SplitInclusiveMut<'_, T, P> where P: FnMut(&T) -> bool {} /// An iterator over subslices separated by elements that match a predicate diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 2c11d5cd25759..c517286d49898 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -3335,7 +3335,7 @@ impl str { /// .split_inclusive('\n').collect(); /// assert_eq!(v, ["Mary had a little lamb\n", "little lamb\n", "little lamb.\n"]); /// ``` - #[unstable(feature = "split_inclusive", issue = "none")] + #[unstable(feature = "split_inclusive", issue = "72360")] #[inline] pub fn split_inclusive<'a, P: Pattern<'a>>(&'a self, pat: P) -> SplitInclusive<'a, P> { SplitInclusive(SplitInternal { @@ -4575,7 +4575,7 @@ pub struct SplitAsciiWhitespace<'a> { /// /// [`split_inclusive`]: ../../std/primitive.str.html#method.split_inclusive /// [`str`]: ../../std/primitive.str.html -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] pub struct SplitInclusive<'a, P: Pattern<'a>>(SplitInternal<'a, P>); impl_fn_for_zst! { @@ -4668,7 +4668,7 @@ impl<'a> DoubleEndedIterator for SplitAsciiWhitespace<'a> { #[stable(feature = "split_ascii_whitespace", since = "1.34.0")] impl FusedIterator for SplitAsciiWhitespace<'_> {} -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, P: Pattern<'a>> Iterator for SplitInclusive<'a, P> { type Item = &'a str; @@ -4678,7 +4678,7 @@ impl<'a, P: Pattern<'a>> Iterator for SplitInclusive<'a, P> { } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, P: Pattern<'a, Searcher: fmt::Debug>> fmt::Debug for SplitInclusive<'a, P> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("SplitInclusive").field("0", &self.0).finish() @@ -4686,14 +4686,14 @@ impl<'a, P: Pattern<'a, Searcher: fmt::Debug>> fmt::Debug for SplitInclusive<'a, } // FIXME(#26925) Remove in favor of `#[derive(Clone)]` -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, P: Pattern<'a, Searcher: Clone>> Clone for SplitInclusive<'a, P> { fn clone(&self) -> Self { SplitInclusive(self.0.clone()) } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, P: Pattern<'a, Searcher: ReverseSearcher<'a>>> DoubleEndedIterator for SplitInclusive<'a, P> { @@ -4703,7 +4703,7 @@ impl<'a, P: Pattern<'a, Searcher: ReverseSearcher<'a>>> DoubleEndedIterator } } -#[unstable(feature = "split_inclusive", issue = "none")] +#[unstable(feature = "split_inclusive", issue = "72360")] impl<'a, P: Pattern<'a>> FusedIterator for SplitInclusive<'a, P> {} /// An iterator of [`u16`] over the string encoded as UTF-16.