From de959afab5042c44d78703050353cb5b20d442d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 3 Nov 2017 17:30:14 -0700 Subject: [PATCH 1/2] Handle anon lifetime arg being returned with named lifetime return type When there's a lifetime mismatch between an argument with an anonymous lifetime being returned in a method with a return type that has a named lifetime, show specialized lifetime error pointing at argument with a hint to give it an explicit lifetime matching the return type. ``` error[E0621]: explicit lifetime required in the type of `other` --> file2.rs:21:21 | 17 | fn bar(&self, other: Foo) -> Foo<'a> { | ----- consider changing the type of `other` to `Foo<'a>` ... 21 | other | ^^^^^ lifetime `'a` required ``` Follow up to #44124 and #42669. --- .../error_reporting/different_lifetimes.rs | 55 ++++++++++++------- .../error_reporting/named_anon_conflict.rs | 16 ++---- src/librustc/infer/error_reporting/util.rs | 1 + .../regions-infer-at-fn-not-param.rs | 2 +- ...oth-anon-regions-earlybound-regions.stderr | 6 +- .../lifetime-errors/ex4-anon-named-regions.rs | 30 ++++++++++ .../ex4-anon-named-regions.stderr | 11 ++++ 7 files changed, 87 insertions(+), 34 deletions(-) create mode 100644 src/test/ui/lifetime-errors/ex4-anon-named-regions.rs create mode 100644 src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr diff --git a/src/librustc/infer/error_reporting/different_lifetimes.rs b/src/librustc/infer/error_reporting/different_lifetimes.rs index ee30db26255..d7e0877d95c 100644 --- a/src/librustc/infer/error_reporting/different_lifetimes.rs +++ b/src/librustc/infer/error_reporting/different_lifetimes.rs @@ -21,25 +21,42 @@ use hir::intravisit::{self, Visitor, NestedVisitorMap}; use infer::error_reporting::util::AnonymousArgInfo; impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { - // This method prints the error message for lifetime errors when both the concerned regions - // are anonymous. - // Consider a case where we have - // fn foo(x: &mut Vec<&u8>, y: &u8) - // { x.push(y); }. - // The example gives - // fn foo(x: &mut Vec<&u8>, y: &u8) { - // --- --- these references are declared with different lifetimes... - // x.push(y); - // ^ ...but data from `y` flows into `x` here - // It has been extended for the case of structs too. - // Consider the example - // struct Ref<'a> { x: &'a u32 } - // fn foo(mut x: Vec, y: Ref) { - // --- --- these structs are declared with different lifetimes... - // x.push(y); - // ^ ...but data from `y` flows into `x` here - // } - // It will later be extended to trait objects. + /// Print the error message for lifetime errors when both the concerned regions are anonymous. + /// + /// Consider a case where we have + /// + /// ```no_run + /// fn foo(x: &mut Vec<&u8>, y: &u8) { + /// x.push(y); + /// } + /// ``` + /// + /// The example gives + /// + /// ```text + /// fn foo(x: &mut Vec<&u8>, y: &u8) { + /// --- --- these references are declared with different lifetimes... + /// x.push(y); + /// ^ ...but data from `y` flows into `x` here + /// ``` + /// + /// It has been extended for the case of structs too. + /// + /// Consider the example + /// + /// ```no_run + /// struct Ref<'a> { x: &'a u32 } + /// ``` + /// + /// ```text + /// fn foo(mut x: Vec, y: Ref) { + /// --- --- these structs are declared with different lifetimes... + /// x.push(y); + /// ^ ...but data from `y` flows into `x` here + /// } + /// ```` + /// + /// It will later be extended to trait objects. pub fn try_report_anon_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool { let (span, sub, sup) = match *error { ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup), diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index 80fb4ce8e03..4563d955413 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -16,18 +16,15 @@ use infer::region_inference::RegionResolutionError; use ty; impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { - // This method generates the error message for the case when - // the function arguments consist of a named region and an anonymous - // region and corresponds to `ConcreteFailure(..)` + /// Generate an error message for when the function arguments consist of a named region and + /// an anonymous region and corresponds to `ConcreteFailure(..)` pub fn try_report_named_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool { let (span, sub, sup) = match *error { ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup), _ => return false, // inapplicable }; - debug!("try_report_named_anon_conflict(sub={:?}, sup={:?})", - sub, - sup); + debug!("try_report_named_anon_conflict(sub={:?}, sup={:?})", sub, sup); // Determine whether the sub and sup consist of one named region ('a) // and one anonymous (elided) region. If so, find the parameter arg @@ -53,10 +50,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { }; debug!("try_report_named_anon_conflict: named = {:?}", named); - debug!("try_report_named_anon_conflict: anon_arg_info = {:?}", - anon_arg_info); - debug!("try_report_named_anon_conflict: region_info = {:?}", - region_info); + debug!("try_report_named_anon_conflict: anon_arg_info = {:?}", anon_arg_info); + debug!("try_report_named_anon_conflict: region_info = {:?}", region_info); let (arg, new_ty, br, is_first, scope_def_id, is_impl_item) = (anon_arg_info.arg, anon_arg_info.arg_ty, @@ -101,6 +96,5 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { .span_label(span, format!("lifetime `{}` required", named)) .emit(); return true; - } } diff --git a/src/librustc/infer/error_reporting/util.rs b/src/librustc/infer/error_reporting/util.rs index 47db3f1b792..6bcd98a7a68 100644 --- a/src/librustc/infer/error_reporting/util.rs +++ b/src/librustc/infer/error_reporting/util.rs @@ -221,6 +221,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { _ => false, } } + ty::ReEarlyBound(_) => true, _ => false, } } diff --git a/src/test/compile-fail/regions-infer-at-fn-not-param.rs b/src/test/compile-fail/regions-infer-at-fn-not-param.rs index 0c250e38258..ec73bf90b6e 100644 --- a/src/test/compile-fail/regions-infer-at-fn-not-param.rs +++ b/src/test/compile-fail/regions-infer-at-fn-not-param.rs @@ -21,7 +21,7 @@ struct not_parameterized2 { } fn take1<'a>(p: parameterized1) -> parameterized1<'a> { p } -//~^ ERROR mismatched types +//~^ ERROR explicit lifetime required in the type of `p` fn take3(p: not_parameterized1) -> not_parameterized1 { p } fn take4(p: not_parameterized2) -> not_parameterized2 { p } diff --git a/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr b/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr index 58f2cb94cec..6c5ed672604 100644 --- a/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr +++ b/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr @@ -1,11 +1,11 @@ -error[E0623]: lifetime mismatch +error[E0621]: explicit lifetime required in the type of `y` --> $DIR/ex3-both-anon-regions-earlybound-regions.rs:17:12 | 13 | fn baz<'a, 'b, T>(x: &mut Vec<&'a T>, y: &T) - | ----- -- these two types are declared with different lifetimes... + | - consider changing the type of `y` to `&'a T` ... 17 | x.push(y); - | ^ ...but data from `y` flows into `x` here + | ^ lifetime `'a` required error: aborting due to previous error diff --git a/src/test/ui/lifetime-errors/ex4-anon-named-regions.rs b/src/test/ui/lifetime-errors/ex4-anon-named-regions.rs new file mode 100644 index 00000000000..55752f753ef --- /dev/null +++ b/src/test/ui/lifetime-errors/ex4-anon-named-regions.rs @@ -0,0 +1,30 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[derive(Clone)] +enum Foo<'a> { + Bar(&'a str), +} + +impl<'a> Foo<'a> { + fn bar(&self, other: Foo) -> Foo<'a> { + match *self { + Foo::Bar(s) => { + if s == "test" { + other + } else { + self.clone() + } + } + } + } +} + +fn main() { } diff --git a/src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr b/src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr new file mode 100644 index 00000000000..4a127bbc551 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr @@ -0,0 +1,11 @@ +error[E0621]: explicit lifetime required in the type of `other` + --> $DIR/ex4-anon-named-regions.rs:21:21 + | +17 | fn bar(&self, other: Foo) -> Foo<'a> { + | ----- consider changing the type of `other` to `Foo<'a>` +... +21 | other + | ^^^^^ lifetime `'a` required + +error: aborting due to previous error + From 805333b2b5602e421124787c7f944c4a7eacdb62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 6 Nov 2017 21:02:31 -0800 Subject: [PATCH 2/2] review comments --- src/librustc/infer/error_reporting/named_anon_conflict.rs | 4 ++-- ... => ex1-return-one-existing-name-early-bound-in-struct.rs} | 0 ...ex1-return-one-existing-name-early-bound-in-struct.stderr} | 2 +- ...-regions.rs => ex2a-push-one-existing-name-early-bound.rs} | 0 ....stderr => ex2a-push-one-existing-name-early-bound.stderr} | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename src/test/ui/lifetime-errors/{ex4-anon-named-regions.rs => ex1-return-one-existing-name-early-bound-in-struct.rs} (100%) rename src/test/ui/lifetime-errors/{ex4-anon-named-regions.stderr => ex1-return-one-existing-name-early-bound-in-struct.stderr} (82%) rename src/test/ui/lifetime-errors/{ex3-both-anon-regions-earlybound-regions.rs => ex2a-push-one-existing-name-early-bound.rs} (100%) rename src/test/ui/lifetime-errors/{ex3-both-anon-regions-earlybound-regions.stderr => ex2a-push-one-existing-name-early-bound.stderr} (83%) diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index 4563d955413..6d3b9507840 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -16,8 +16,8 @@ use infer::region_inference::RegionResolutionError; use ty; impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { - /// Generate an error message for when the function arguments consist of a named region and - /// an anonymous region and corresponds to `ConcreteFailure(..)` + /// When given a `ConcreteFailure` for a function with arguments containing a named region and + /// an anonymous region, emit an descriptive diagnostic error. pub fn try_report_named_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool { let (span, sub, sup) = match *error { ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup), diff --git a/src/test/ui/lifetime-errors/ex4-anon-named-regions.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-early-bound-in-struct.rs similarity index 100% rename from src/test/ui/lifetime-errors/ex4-anon-named-regions.rs rename to src/test/ui/lifetime-errors/ex1-return-one-existing-name-early-bound-in-struct.rs diff --git a/src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-early-bound-in-struct.stderr similarity index 82% rename from src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr rename to src/test/ui/lifetime-errors/ex1-return-one-existing-name-early-bound-in-struct.stderr index 4a127bbc551..d1660a620b6 100644 --- a/src/test/ui/lifetime-errors/ex4-anon-named-regions.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-early-bound-in-struct.stderr @@ -1,5 +1,5 @@ error[E0621]: explicit lifetime required in the type of `other` - --> $DIR/ex4-anon-named-regions.rs:21:21 + --> $DIR/ex1-return-one-existing-name-early-bound-in-struct.rs:21:21 | 17 | fn bar(&self, other: Foo) -> Foo<'a> { | ----- consider changing the type of `other` to `Foo<'a>` diff --git a/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.rs b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-early-bound.rs similarity index 100% rename from src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.rs rename to src/test/ui/lifetime-errors/ex2a-push-one-existing-name-early-bound.rs diff --git a/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-early-bound.stderr similarity index 83% rename from src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr rename to src/test/ui/lifetime-errors/ex2a-push-one-existing-name-early-bound.stderr index 6c5ed672604..980f14a51d9 100644 --- a/src/test/ui/lifetime-errors/ex3-both-anon-regions-earlybound-regions.stderr +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-early-bound.stderr @@ -1,5 +1,5 @@ error[E0621]: explicit lifetime required in the type of `y` - --> $DIR/ex3-both-anon-regions-earlybound-regions.rs:17:12 + --> $DIR/ex2a-push-one-existing-name-early-bound.rs:17:12 | 13 | fn baz<'a, 'b, T>(x: &mut Vec<&'a T>, y: &T) | - consider changing the type of `y` to `&'a T`