From: David Wood Date: Tue, 18 Sep 2018 19:28:06 +0000 (+0200) Subject: Correctly handle named lifetimes. X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=ef10e94993f6e4b04b283e414b04ed02ec9f6c99;p=rust.git Correctly handle named lifetimes. Enhances annotation logic to properly consider named lifetimes where lifetime elision rules that were previously implemented would not apply. Further, adds new help and note messages to diagnostics and highlights only lifetime when dealing with named lifetimes. --- diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index ea547c592d0..8fb5ed66a82 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -1324,6 +1324,23 @@ pub fn shifted_out_to_binder(self, to_binder: DebruijnIndex) -> Self { /// Region utilities impl RegionKind { + /// Is this region named by the user? + pub fn has_name(&self) -> bool { + match *self { + RegionKind::ReEarlyBound(ebr) => ebr.has_name(), + RegionKind::ReLateBound(_, br) => br.is_named(), + RegionKind::ReFree(fr) => fr.bound_region.is_named(), + RegionKind::ReScope(..) => false, + RegionKind::ReStatic => true, + RegionKind::ReVar(..) => false, + RegionKind::ReSkolemized(_, br) => br.is_named(), + RegionKind::ReEmpty => false, + RegionKind::ReErased => false, + RegionKind::ReClosureBound(..) => false, + RegionKind::ReCanonical(..) => false, + } + } + pub fn is_late_bound(&self) -> bool { match *self { ty::ReLateBound(..) => true, diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 73fdb4bd5d1..cc1203a236d 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -17,6 +17,7 @@ LocalDecl, LocalKind, Location, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, VarBindingForm, }; +use rustc::hir; use rustc::hir::def_id::DefId; use rustc::ty; use rustc_data_structures::fx::FxHashSet; @@ -524,10 +525,8 @@ fn report_local_value_does_not_live_long_enough( err.span_label( drop_span, format!( - "...but `{}` is only valid for the duration of the `{}` function, so it \ - is dropped here while still borrowed", - name, - self.infcx.tcx.hir.name(fn_node_id), + "but `{}` will be dropped here, when the function `{}` returns", + name, self.infcx.tcx.hir.name(fn_node_id), ) ); @@ -1235,67 +1234,137 @@ fn annotate_fn_sig( let fn_node_id = self.infcx.tcx.hir.as_local_node_id(did)?; let fn_decl = self.infcx.tcx.hir.fn_decl(fn_node_id)?; - // If there is one argument and this error is being reported, that means - // that the lifetime of the borrow could not be made to match the single - // argument's lifetime. We can highlight it. + // We need to work out which arguments to highlight. We do this by looking + // at the return type, where there are three cases: // - // If there is more than one argument and this error is being reported, that - // means there must be a self parameter - as otherwise there would be an error - // from lifetime elision and not this. So we highlight the self parameter. - let argument_span = fn_decl.inputs.first()?.span; - let argument_ty = sig.inputs().skip_binder().first()?; - if is_closure { - // Closure arguments are wrapped in a tuple, so we need to get the first - // from that. - let argument_ty = if let ty::TyKind::Tuple(elems) = argument_ty.sty { - let argument_ty = elems.first()?; - if let ty::TyKind::Ref(_, _, _) = argument_ty.sty { - argument_ty - } else { + // 1. If there are named arguments, then we should highlight the return type and + // highlight any of the arguments that are also references with that lifetime. + // If there are no arguments that have the same lifetime as the return type, + // then don't highlight anything. + // 2. The return type is a reference with an anonymous lifetime. If this is + // the case, then we can take advantage of (and teach) the lifetime elision + // rules. + // + // We know that an error is being reported. So the arguments and return type + // must satisfy the elision rules. Therefore, if there is a single argument + // then that means the return type and first (and only) argument have the same + // lifetime and the borrow isn't meeting that, we can highlight the argument + // and return type. + // + // If there are multiple arguments then the first argument must be self (else + // it would not satisfy the elision rules), so we can highlight self and the + // return type. + // 3. The return type is not a reference. In this case, we don't highlight + // anything. + let return_ty = sig.output(); + match return_ty.skip_binder().sty { + ty::TyKind::Ref(return_region, _, _) if return_region.has_name() && !is_closure => { + // This is case 1 from above, return type is a named reference so we need to + // search for relevant arguments. + let mut arguments = Vec::new(); + for (index, argument) in sig.inputs().skip_binder().iter().enumerate() { + if let ty::TyKind::Ref(argument_region, _, _) = argument.sty { + if argument_region == return_region { + // Need to use the `rustc::ty` types to compare against the + // `return_region`. Then use the `rustc::hir` type to get only + // the lifetime span. + match &fn_decl.inputs[index].node { + hir::TyKind::Rptr(lifetime, _) => { + // With access to the lifetime, we can get + // the span of it. + arguments.push((*argument, lifetime.span)); + }, + _ => bug!("ty type is a ref but hir type is not"), + } + } + } + } + + // We need to have arguments. This shouldn't happen, but it's worth checking. + if arguments.is_empty() { return None; } - } else { - return None - }; - Some(AnnotatedBorrowFnSignature::Closure { - argument_ty, - argument_span, - }) - } else if let ty::TyKind::Ref(argument_region, _, _) = argument_ty.sty { - // We only consider the return type for functions. - let return_span = fn_decl.output.span(); - - let return_ty = sig.output(); - let (return_region, return_ty) = if let ty::TyKind::Ref( - return_region, _, _ - ) = return_ty.skip_binder().sty { - (return_region, *return_ty.skip_binder()) - } else { - return None; - }; + // We use a mix of the HIR and the Ty types to get information + // as the HIR doesn't have full types for closure arguments. + let return_ty = *sig.output().skip_binder(); + let mut return_span = fn_decl.output.span(); + if let hir::FunctionRetTy::Return(ty) = fn_decl.output { + if let hir::TyKind::Rptr(lifetime, _) = ty.into_inner().node { + return_span = lifetime.span; + } + } - Some(AnnotatedBorrowFnSignature::Function { - argument_ty, - argument_span, - return_ty, - return_span, - regions_equal: return_region == argument_region, - }) - } else { - None + Some(AnnotatedBorrowFnSignature::NamedFunction { + arguments, + return_ty, + return_span, + }) + }, + ty::TyKind::Ref(_, _, _) if is_closure => { + // This is case 2 from above but only for closures, return type is anonymous + // reference so we select + // the first argument. + let argument_span = fn_decl.inputs.first()?.span; + let argument_ty = sig.inputs().skip_binder().first()?; + + // Closure arguments are wrapped in a tuple, so we need to get the first + // from that. + if let ty::TyKind::Tuple(elems) = argument_ty.sty { + let argument_ty = elems.first()?; + if let ty::TyKind::Ref(_, _, _) = argument_ty.sty { + return Some(AnnotatedBorrowFnSignature::Closure { + argument_ty, + argument_span, + }); + } + } + + None + }, + ty::TyKind::Ref(_, _, _) => { + // This is also case 2 from above but for functions, return type is still an + // anonymous reference so we select the first argument. + let argument_span = fn_decl.inputs.first()?.span; + let argument_ty = sig.inputs().skip_binder().first()?; + + let return_span = fn_decl.output.span(); + let return_ty = *sig.output().skip_binder(); + + // We expect the first argument to be a reference. + match argument_ty.sty { + ty::TyKind::Ref(_, _, _) => {}, + _ => return None, + } + + Some(AnnotatedBorrowFnSignature::AnonymousFunction { + argument_ty, + argument_span, + return_ty, + return_span, + }) + }, + _ => { + // This is case 3 from above, return type is not a reference so don't highlight + // anything. + None + }, } } } #[derive(Debug)] enum AnnotatedBorrowFnSignature<'tcx> { - Function { + NamedFunction { + arguments: Vec<(ty::Ty<'tcx>, Span)>, + return_ty: ty::Ty<'tcx>, + return_span: Span, + }, + AnonymousFunction { argument_ty: ty::Ty<'tcx>, argument_span: Span, return_ty: ty::Ty<'tcx>, return_span: Span, - regions_equal: bool, }, Closure { argument_ty: ty::Ty<'tcx>, @@ -1304,57 +1373,86 @@ enum AnnotatedBorrowFnSignature<'tcx> { } impl<'tcx> AnnotatedBorrowFnSignature<'tcx> { + /// Annotate the provided diagnostic with information about borrow from the fn signature that + /// helps explain. fn emit( &self, diag: &mut DiagnosticBuilder<'_> ) -> String { - let (argument_ty, argument_span) = match self { - AnnotatedBorrowFnSignature::Function { - argument_ty, - argument_span, - .. - } => (argument_ty, argument_span), - AnnotatedBorrowFnSignature::Closure { + match self { + AnnotatedBorrowFnSignature::Closure { argument_ty, argument_span } => { + diag.span_label( + *argument_span, + format!("has type `{}`", self.get_name_for_ty(argument_ty, 0)), + ); + + self.get_region_name_for_ty(argument_ty, 0) + }, + AnnotatedBorrowFnSignature::AnonymousFunction { argument_ty, argument_span, - } => (argument_ty, argument_span), - }; + return_ty, + return_span, + } => { + let argument_ty_name = self.get_name_for_ty(argument_ty, 0); + diag.span_label( + *argument_span, + format!("has type `{}`", argument_ty_name) + ); - let (argument_region_name, argument_ty_name) = ( - self.get_region_name_for_ty(argument_ty, 0), - self.get_name_for_ty(argument_ty, 0), - ); - diag.span_label( - *argument_span, - format!("has type `{}`", argument_ty_name) - ); + let return_ty_name = self.get_name_for_ty(return_ty, 0); + let types_equal = return_ty_name == argument_ty_name; + diag.span_label( + *return_span, + format!( + "{}has type `{}`", + if types_equal { "also " } else { "" }, + return_ty_name, + ) + ); - // Only emit labels for the return value when we're annotating a function. - if let AnnotatedBorrowFnSignature::Function { - return_ty, - return_span, - regions_equal, - .. - } = self { - let counter = if *regions_equal { 0 } else { 1 }; - let (return_region_name, return_ty_name) = ( - self.get_region_name_for_ty(return_ty, counter), - self.get_name_for_ty(return_ty, counter) - ); + diag.note( + "argument and return type have the same lifetime due to lifetime elision rules", + ); + diag.note( + "to learn more, visit ", + ); - let types_equal = return_ty_name == argument_ty_name; - diag.span_label( - *return_span, - format!( - "{}has type `{}`", - if types_equal { "also " } else { "" }, - return_ty_name, - ) - ); + self.get_region_name_for_ty(return_ty, 0) + }, + AnnotatedBorrowFnSignature::NamedFunction { + arguments, + return_ty, + return_span, + } => { + // Region of return type and arguments checked to be the same earlier. + let region_name = self.get_region_name_for_ty(return_ty, 0); + for (_, argument_span) in arguments { + diag.span_label( + *argument_span, + format!("has lifetime `{}`", region_name) + ); + } - return_region_name - } else { - argument_region_name + diag.span_label( + *return_span, + format!( + "also has lifetime `{}`", + region_name, + ) + ); + + diag.help( + &format!( + "use data from the highlighted arguments which match the `{}` lifetime of \ + the return type", + region_name, + ), + ); + + region_name + }, } } diff --git a/src/test/ui/issues/issue-30438-c.nll.stderr b/src/test/ui/issues/issue-30438-c.nll.stderr index e7f2efcb01a..71615333e64 100644 --- a/src/test/ui/issues/issue-30438-c.nll.stderr +++ b/src/test/ui/issues/issue-30438-c.nll.stderr @@ -2,16 +2,17 @@ error[E0597]: `x` does not live long enough --> $DIR/issue-30438-c.rs:19:5 | LL | fn silly<'y, 'z>(_s: &'y Test<'z>) -> &'y as Trait>::Out where 'z: 'static { - | ------------ ---------------------------- has type `&'y as Trait>::Out` - | | - | has type `&'y Test<'z>` + | -- -- also has lifetime `'y` + | | + | has lifetime `'y` LL | let x = Test { s: "this cannot last" }; LL | &x | ^^ `x` would have to be valid for `'y` LL | //~^ ERROR: `x` does not live long enough LL | } - | - ...but `x` is only valid for the duration of the `silly` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `silly` returns | + = help: use data from the highlighted arguments which match the `'y` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/nll/borrowed-universal-error-2.stderr b/src/test/ui/nll/borrowed-universal-error-2.stderr index cfa46881ae0..ae884c1a943 100644 --- a/src/test/ui/nll/borrowed-universal-error-2.stderr +++ b/src/test/ui/nll/borrowed-universal-error-2.stderr @@ -2,16 +2,17 @@ error[E0597]: `v` does not live long enough --> $DIR/borrowed-universal-error-2.rs:16:5 | LL | fn foo<'a>(x: &'a (u32,)) -> &'a u32 { - | ---------- ------- has type `&'a u32` - | | - | has type `&'a (u32,)` + | -- -- also has lifetime `'a` + | | + | has lifetime `'a` LL | let v = 22; LL | &v | ^^ `v` would have to be valid for `'a` LL | //~^ ERROR `v` does not live long enough [E0597] LL | } - | - ...but `v` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed + | - but `v` will be dropped here, when the function `foo` returns | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/nll/issue-52534-1.rs b/src/test/ui/nll/issue-52534-1.rs index a6f3f9fbd3c..cd6c10335cc 100644 --- a/src/test/ui/nll/issue-52534-1.rs +++ b/src/test/ui/nll/issue-52534-1.rs @@ -30,4 +30,24 @@ fn baz(x: &u32) -> &&u32 { &&x } +fn foobazbar<'a>(x: u32, y: &'a u32) -> &'a u32 { + let x = 22; + &x +} + +fn foobar<'a>(x: &'a u32) -> &'a u32 { + let x = 22; + &x +} + +fn foobaz<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { + let x = 22; + &x +} + +fn foobarbaz<'a, 'b>(x: &'a u32, y: &'b u32, z: &'a u32) -> &'a u32 { + let x = 22; + &x +} + fn main() { } diff --git a/src/test/ui/nll/issue-52534-1.stderr b/src/test/ui/nll/issue-52534-1.stderr index 2400cb72a38..0f492884b83 100644 --- a/src/test/ui/nll/issue-52534-1.stderr +++ b/src/test/ui/nll/issue-52534-1.stderr @@ -9,8 +9,10 @@ LL | let x = 22; LL | &x | ^^ `x` would have to be valid for `'0` LL | } - | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `bar` returns | + = note: argument and return type have the same lifetime due to lifetime elision rules + = note: to learn more, visit = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -25,8 +27,10 @@ LL | let x = 22; LL | &x | ^^ `x` would have to be valid for `'0` LL | } - | - ...but `x` is only valid for the duration of the `foo` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `foo` returns | + = note: argument and return type have the same lifetime due to lifetime elision rules + = note: to learn more, visit = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -41,8 +45,10 @@ LL | let x = 22; LL | &&x | ^^ `x` would have to be valid for `'0` LL | } - | - ...but `x` is only valid for the duration of the `baz` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `baz` returns | + = note: argument and return type have the same lifetime due to lifetime elision rules + = note: to learn more, visit = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -63,6 +69,72 @@ LL | | &&x LL | | } | |_^ -error: aborting due to 4 previous errors +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:35:5 + | +LL | fn foobazbar<'a>(x: u32, y: &'a u32) -> &'a u32 { + | -- -- also has lifetime `'a` + | | + | has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobazbar` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:40:5 + | +LL | fn foobar<'a>(x: &'a u32) -> &'a u32 { + | -- -- also has lifetime `'a` + | | + | has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobar` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:45:5 + | +LL | fn foobaz<'a, 'b>(x: &'a u32, y: &'b u32) -> &'a u32 { + | -- has lifetime `'a` -- also has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobaz` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error[E0597]: `x` does not live long enough + --> $DIR/issue-52534-1.rs:50:5 + | +LL | fn foobarbaz<'a, 'b>(x: &'a u32, y: &'b u32, z: &'a u32) -> &'a u32 { + | -- -- -- also has lifetime `'a` + | | | + | has lifetime `'a` has lifetime `'a` +LL | let x = 22; +LL | &x + | ^^ `x` would have to be valid for `'a` +LL | } + | - but `x` will be dropped here, when the function `foobarbaz` returns + | + = help: use data from the highlighted arguments which match the `'a` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error: aborting due to 8 previous errors For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-52534.stderr b/src/test/ui/nll/issue-52534.stderr index fc7766885f5..fafa216842d 100644 --- a/src/test/ui/nll/issue-52534.stderr +++ b/src/test/ui/nll/issue-52534.stderr @@ -6,7 +6,7 @@ LL | foo(|a| &x) | | | has type `&'0 u32` LL | } - | - ...but `x` is only valid for the duration of the `bar` function, so it is dropped here while still borrowed + | - but `x` will be dropped here, when the function `bar` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit @@ -19,7 +19,7 @@ LL | baz(|first, second| &y) | | | has type `&'0 u32` LL | } - | - ...but `y` is only valid for the duration of the `foobar` function, so it is dropped here while still borrowed + | - but `y` will be dropped here, when the function `foobar` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/regions/regions-nested-fns-2.nll.stderr b/src/test/ui/regions/regions-nested-fns-2.nll.stderr index 62c4a6eae94..30bdbad4113 100644 --- a/src/test/ui/regions/regions-nested-fns-2.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns-2.nll.stderr @@ -8,7 +8,7 @@ LL | if false { &y } else { z } | ^ `y` would have to be valid for `'0` LL | }); LL | } - | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed + | - but `y` will be dropped here, when the function `nested` returns | = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments = note: to learn more, visit diff --git a/src/test/ui/regions/regions-nested-fns.nll.stderr b/src/test/ui/regions/regions-nested-fns.nll.stderr index b53ea459d6c..4bb602d572f 100644 --- a/src/test/ui/regions/regions-nested-fns.nll.stderr +++ b/src/test/ui/regions/regions-nested-fns.nll.stderr @@ -25,16 +25,15 @@ error[E0597]: `y` does not live long enough --> $DIR/regions-nested-fns.rs:19:15 | LL | ignore:: FnMut(&'z isize)>>(Box::new(|z| { - | - has type `&'0 isize` + | --- value captured here LL | ay = x; LL | ay = &y; - | ^ `y` would have to be valid for `'0` + | ^ borrowed value does not live long enough ... LL | } - | - ...but `y` is only valid for the duration of the `nested` function, so it is dropped here while still borrowed + | - `y` dropped here while still borrowed | - = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments - = note: to learn more, visit + = note: borrowed value must be valid for the static lifetime... error: unsatisfied lifetime constraints --> $DIR/regions-nested-fns.rs:23:68