From f72b8a44c51313d384deefcda753df668f2e265e Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 5 Aug 2018 14:33:38 +0100 Subject: [PATCH 1/2] Use span of the closure args in free region errors --- .../error_reporting/region_name.rs | 62 ++++++++++++++++--- ...1-does-not-trigger-for-closures.nll.stderr | 5 +- src/test/ui/issue-40510-1.nll.stderr | 20 +++--- src/test/ui/issue-40510-3.nll.stderr | 25 ++++---- src/test/ui/issue-49824.nll.stderr | 25 ++++---- src/test/ui/nll/issue-48238.stderr | 9 +-- 6 files changed, 91 insertions(+), 55 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index 79165276430..6a21e002588 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -10,6 +10,7 @@ use borrow_check::nll::region_infer::RegionInferenceContext; use borrow_check::nll::ToRegionVid; +use borrow_check::nll::universal_regions::DefiningTy; use rustc::hir; use rustc::hir::def_id::DefId; use rustc::infer::InferCtxt; @@ -72,7 +73,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { }) .or_else(|| { self.give_name_if_anonymous_region_appears_in_output( - infcx.tcx, mir, fr, counter, diag) + infcx.tcx, mir, mir_def_id, fr, counter, diag) }) .unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr)) } @@ -107,13 +108,46 @@ impl<'tcx> RegionInferenceContext<'tcx> { }, ty::BoundRegion::BrEnv => { - let closure_span = tcx.hir.span_if_local(mir_def_id).unwrap(); - let region_name = self.synthesize_region_name(counter); - diag.span_label( - closure_span, - format!("lifetime `{}` represents the closure body", region_name), - ); - Some(region_name) + let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir"); + let def_ty = self.universal_regions.defining_ty; + + if let DefiningTy::Closure(def_id, substs) = def_ty { + let args_span = if let hir::ExprKind::Closure(_, _, _, span, _) + = tcx.hir.expect_expr(mir_node_id).node + { + span + } else { + bug!("Closure is not defined by a closure expr"); + }; + let region_name = self.synthesize_region_name(counter); + diag.span_label( + args_span, + format!("lifetime `{}` represents this closure's body", region_name), + ); + + let closure_kind_ty = substs.closure_kind_ty(def_id, tcx); + let note = match closure_kind_ty.to_opt_closure_kind() { + Some(ty::ClosureKind::Fn) => { + "closure implements `Fn`, so references to captured variables \ + can't escape the closure" + } + Some(ty::ClosureKind::FnMut) => { + "closure implements `FnMut`, so references to captured variables \ + can't escape the closure" + } + Some(ty::ClosureKind::FnOnce) => { + bug!("BrEnv in a `FnOnce` closure"); + } + None => bug!("Closure kind not inferred in borrow check"), + }; + + diag.note(note); + + Some(region_name) + } else { + // Can't have BrEnv in functions, constants or generators. + bug!("BrEnv outside of closure."); + } } ty::BoundRegion::BrAnon(_) | ty::BoundRegion::BrFresh(_) => None, @@ -545,6 +579,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, tcx: TyCtxt<'_, '_, 'tcx>, mir: &Mir<'tcx>, + mir_def_id: DefId, fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, @@ -558,9 +593,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { return None; } + let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir"); + let args_span = if let hir::ExprKind::Closure(_, _, _, span, _) + = tcx.hir.expect_expr(mir_node_id).node + { + span + } else { + mir.span + }; + let region_name = self.synthesize_region_name(counter); diag.span_label( - mir.span, + args_span, format!("lifetime `{}` appears in return type", region_name), ); diff --git a/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr b/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr index 0ac295c54bc..27a51cb83fb 100644 --- a/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr +++ b/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr @@ -8,9 +8,8 @@ error: unsatisfied lifetime constraints --> $DIR/E0621-does-not-trigger-for-closures.rs:25:26 | LL | invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495 - | ----------^^^^^----------------- - | | | | - | | | requires that `'1` must outlive `'2` + | ------ ^^^^^ requires that `'1` must outlive `'2` + | | | | | has type `&'1 i32` | lifetime `'2` appears in return type diff --git a/src/test/ui/issue-40510-1.nll.stderr b/src/test/ui/issue-40510-1.nll.stderr index 312ec6e742e..7eb6a0e7fb2 100644 --- a/src/test/ui/issue-40510-1.nll.stderr +++ b/src/test/ui/issue-40510-1.nll.stderr @@ -1,17 +1,15 @@ error: unsatisfied lifetime constraints --> $DIR/issue-40510-1.rs:18:9 | -LL | || { - | _____- - | |_____| - | || -LL | || &mut x - | || ^^^^^^ return requires that `'1` must outlive `'2` -LL | || }; - | || - - | ||_____| - | |______lifetime `'1` represents the closure body - | lifetime `'2` appears in return type +LL | || { + | -- + | | + | lifetime `'1` represents this closure's body + | lifetime `'2` appears in return type +LL | &mut x + | ^^^^^^ return requires that `'1` must outlive `'2` + | + = note: closure implements `FnMut`, so references to captured variables can't escape the closure error: aborting due to previous error diff --git a/src/test/ui/issue-40510-3.nll.stderr b/src/test/ui/issue-40510-3.nll.stderr index eb44850e639..ae3ae3a27ab 100644 --- a/src/test/ui/issue-40510-3.nll.stderr +++ b/src/test/ui/issue-40510-3.nll.stderr @@ -1,20 +1,17 @@ error: unsatisfied lifetime constraints --> $DIR/issue-40510-3.rs:18:9 | -LL | || { - | _____- - | |_____| - | || -LL | || || { - | ||_________^ -LL | ||| x.push(()) -LL | ||| } - | |||_________^ requires that `'1` must outlive `'2` -LL | || }; - | || - - | ||_____| - | |______lifetime `'1` represents the closure body - | lifetime `'2` appears in return type +LL | || { + | -- + | | + | lifetime `'1` represents this closure's body + | lifetime `'2` appears in return type +LL | / || { +LL | | x.push(()) +LL | | } + | |_________^ requires that `'1` must outlive `'2` + | + = note: closure implements `FnMut`, so references to captured variables can't escape the closure error: aborting due to previous error diff --git a/src/test/ui/issue-49824.nll.stderr b/src/test/ui/issue-49824.nll.stderr index 59345754e9f..432036c9d90 100644 --- a/src/test/ui/issue-49824.nll.stderr +++ b/src/test/ui/issue-49824.nll.stderr @@ -1,20 +1,17 @@ error: unsatisfied lifetime constraints --> $DIR/issue-49824.rs:22:9 | -LL | || { - | _____- - | |_____| - | || -LL | || || { - | ||_________^ -LL | ||| let _y = &mut x; -LL | ||| } - | |||_________^ requires that `'1` must outlive `'2` -LL | || }; - | || - - | ||_____| - | |______lifetime `'1` represents the closure body - | lifetime `'2` appears in return type +LL | || { + | -- + | | + | lifetime `'1` represents this closure's body + | lifetime `'2` appears in return type +LL | / || { +LL | | let _y = &mut x; +LL | | } + | |_________^ requires that `'1` must outlive `'2` + | + = note: closure implements `FnMut`, so references to captured variables can't escape the closure error: aborting due to previous error diff --git a/src/test/ui/nll/issue-48238.stderr b/src/test/ui/nll/issue-48238.stderr index 7bdac345e90..4baa9044424 100644 --- a/src/test/ui/nll/issue-48238.stderr +++ b/src/test/ui/nll/issue-48238.stderr @@ -2,11 +2,12 @@ error: unsatisfied lifetime constraints --> $DIR/issue-48238.rs:21:13 | LL | move || use_val(&orig); //~ ERROR - | --------^^^^^^^^^^^^^^ - | | | - | | argument requires that `'1` must outlive `'2` - | lifetime `'1` represents the closure body + | ------- ^^^^^^^^^^^^^^ argument requires that `'1` must outlive `'2` + | | + | lifetime `'1` represents this closure's body | lifetime `'2` appears in return type + | + = note: closure implements `Fn`, so references to captured variables can't escape the closure error: aborting due to previous error From b13e3f87709031be5c599ff23d73f981d04416fd Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 6 Aug 2018 21:42:26 +0100 Subject: [PATCH 2/2] Name return type in free region messages --- .../error_reporting/region_name.rs | 35 +++++++++++++------ ...1-does-not-trigger-for-closures.nll.stderr | 8 ++--- src/test/ui/issue-40510-1.nll.stderr | 4 +-- src/test/ui/issue-40510-3.nll.stderr | 4 +-- src/test/ui/issue-49824.nll.stderr | 4 +-- src/test/ui/nll/issue-48238.stderr | 4 +-- 6 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index 6a21e002588..8c2a5f19038 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -73,7 +73,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { }) .or_else(|| { self.give_name_if_anonymous_region_appears_in_output( - infcx.tcx, mir, mir_def_id, fr, counter, diag) + infcx, mir, mir_def_id, fr, counter, diag) }) .unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr)) } @@ -577,38 +577,51 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// or be early bound (named, not in argument). fn give_name_if_anonymous_region_appears_in_output( &self, - tcx: TyCtxt<'_, '_, 'tcx>, + infcx: &InferCtxt<'_, '_, 'tcx>, mir: &Mir<'tcx>, mir_def_id: DefId, fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, ) -> Option { + let tcx = infcx.tcx; + let return_ty = self.universal_regions.unnormalized_output_ty; debug!( "give_name_if_anonymous_region_appears_in_output: return_ty = {:?}", return_ty ); - if !tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) { + if !infcx.tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) { return None; } - let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir"); - let args_span = if let hir::ExprKind::Closure(_, _, _, span, _) + let type_name = with_highlight_region(fr, *counter, || { + infcx.extract_type_name(&return_ty) + }); + + let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir"); + + let (return_span, mir_description) = if let hir::ExprKind::Closure(_, _, _, span, gen_move) = tcx.hir.expect_expr(mir_node_id).node { - span + ( + tcx.sess.codemap().end_point(span), + if gen_move.is_some() { " of generator" } else { " of closure" } + ) } else { - mir.span + // unreachable? + (mir.span, "") }; - let region_name = self.synthesize_region_name(counter); diag.span_label( - args_span, - format!("lifetime `{}` appears in return type", region_name), + return_span, + format!("return type{} is {}", mir_description, type_name), ); - Some(region_name) + // This counter value will already have been used, so this function will increment it + // so the next value will be used next and return the region name that would have been + // used. + Some(self.synthesize_region_name(counter)) } /// Create a synthetic region named `'1`, incrementing the diff --git a/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr b/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr index 27a51cb83fb..3f9104373d6 100644 --- a/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr +++ b/src/test/ui/error-codes/E0621-does-not-trigger-for-closures.nll.stderr @@ -8,10 +8,10 @@ error: unsatisfied lifetime constraints --> $DIR/E0621-does-not-trigger-for-closures.rs:25:26 | LL | invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495 - | ------ ^^^^^ requires that `'1` must outlive `'2` - | | | - | | has type `&'1 i32` - | lifetime `'2` appears in return type + | -- ^^^^^ requires that `'1` must outlive `'2` + | || + | |return type of closure is &'2 i32 + | has type `&'1 i32` error: aborting due to previous error diff --git a/src/test/ui/issue-40510-1.nll.stderr b/src/test/ui/issue-40510-1.nll.stderr index 7eb6a0e7fb2..6c77bcb2757 100644 --- a/src/test/ui/issue-40510-1.nll.stderr +++ b/src/test/ui/issue-40510-1.nll.stderr @@ -3,9 +3,9 @@ error: unsatisfied lifetime constraints | LL | || { | -- - | | + | || + | |return type of closure is &'2 mut std::boxed::Box<()> | lifetime `'1` represents this closure's body - | lifetime `'2` appears in return type LL | &mut x | ^^^^^^ return requires that `'1` must outlive `'2` | diff --git a/src/test/ui/issue-40510-3.nll.stderr b/src/test/ui/issue-40510-3.nll.stderr index ae3ae3a27ab..8aeef86c2e8 100644 --- a/src/test/ui/issue-40510-3.nll.stderr +++ b/src/test/ui/issue-40510-3.nll.stderr @@ -3,9 +3,9 @@ error: unsatisfied lifetime constraints | LL | || { | -- - | | + | || + | |return type of closure is [closure@$DIR/issue-40510-3.rs:18:9: 20:10 x:&'2 mut std::vec::Vec<()>] | lifetime `'1` represents this closure's body - | lifetime `'2` appears in return type LL | / || { LL | | x.push(()) LL | | } diff --git a/src/test/ui/issue-49824.nll.stderr b/src/test/ui/issue-49824.nll.stderr index 432036c9d90..fb4bed76a71 100644 --- a/src/test/ui/issue-49824.nll.stderr +++ b/src/test/ui/issue-49824.nll.stderr @@ -3,9 +3,9 @@ error: unsatisfied lifetime constraints | LL | || { | -- - | | + | || + | |return type of closure is [closure@$DIR/issue-49824.rs:22:9: 24:10 x:&'2 mut i32] | lifetime `'1` represents this closure's body - | lifetime `'2` appears in return type LL | / || { LL | | let _y = &mut x; LL | | } diff --git a/src/test/ui/nll/issue-48238.stderr b/src/test/ui/nll/issue-48238.stderr index 4baa9044424..84d0730025a 100644 --- a/src/test/ui/nll/issue-48238.stderr +++ b/src/test/ui/nll/issue-48238.stderr @@ -3,9 +3,9 @@ error: unsatisfied lifetime constraints | LL | move || use_val(&orig); //~ ERROR | ------- ^^^^^^^^^^^^^^ argument requires that `'1` must outlive `'2` - | | + | | | + | | return type of closure is &'2 u8 | lifetime `'1` represents this closure's body - | lifetime `'2` appears in return type | = note: closure implements `Fn`, so references to captured variables can't escape the closure