From 176225c4ddd0dd6f469826b40b0cdae399b4a9c3 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Mon, 12 Jun 2017 11:45:19 -0700 Subject: [PATCH 01/17] Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter --- src/librustc/diagnostics.rs | 25 ++++ src/librustc/infer/error_reporting/mod.rs | 41 ++++--- .../error_reporting/named_anon_conflict.rs | 115 ++++++++++++++++++ src/librustc/infer/error_reporting/util.rs | 56 +++++++++ src/librustc/infer/mod.rs | 2 +- .../ex1-return-one-existing-name-if-else-2.rs | 15 +++ ...-return-one-existing-name-if-else-2.stderr | 10 ++ .../ex1-return-one-existing-name-if-else-3.rs | 15 +++ ...-return-one-existing-name-if-else-3.stderr | 10 ++ ...one-existing-name-if-else-using-closure.rs | 24 ++++ ...existing-name-if-else-using-closure.stderr | 29 +++++ ...x1-return-one-existing-name-if-else.stderr | 23 +--- .../ex2a-push-one-existing-name-2.rs | 19 +++ .../ex2a-push-one-existing-name-2.stderr | 10 ++ .../ex2a-push-one-existing-name.stderr | 25 +--- 15 files changed, 365 insertions(+), 54 deletions(-) create mode 100644 src/librustc/infer/error_reporting/named_anon_conflict.rs create mode 100644 src/librustc/infer/error_reporting/util.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr create mode 100644 src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.rs create mode 100644 src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 5e36bd8ec2772..1e7f3f9aeb8b9 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -1946,6 +1946,31 @@ Maybe you just misspelled the lint name or the lint doesn't exist anymore. Either way, try to update/remove it in order to fix the error. "##, +E0611: r##" +Lifetime parameter is missing in one of the function argument. Erroneous +code example: + +```compile_fail,E0611 +fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { // explicit lifetime required + // in the type of `y` + if x > y { x } else { y } +} + +fn main () { } +``` + +Please add the missing lifetime parameter to remove this error. Example: + +``` +fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 { + if x > y { x } else { y } +} + +fn main() { +} +``` +"##, + } diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 11bac21bc429e..1bc01ab858c94 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -67,14 +67,17 @@ use hir::def_id::DefId; use middle::region; use traits::{ObligationCause, ObligationCauseCode}; use ty::{self, TyCtxt, TypeFoldable}; -use ty::{Region, Issue32330}; +use ty::{Region, Issue32330 }; use ty::error::TypeError; use syntax::ast::DUMMY_NODE_ID; use syntax_pos::{Pos, Span}; use errors::{DiagnosticBuilder, DiagnosticStyledString}; - mod note; + mod need_type_info; +mod named_anon_conflict; +mod util; + impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { pub fn note_and_explain_region(self, @@ -255,34 +258,42 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { - pub fn report_region_errors(&self, - errors: &Vec>) { + + pub fn report_region_errors(&self, errors: &Vec>) { debug!("report_region_errors(): {} errors to start", errors.len()); // try to pre-process the errors, which will group some of them // together into a `ProcessedErrors` group: let errors = self.process_errors(errors); - debug!("report_region_errors: {} errors after preprocessing", errors.len()); + debug!("report_region_errors: {} errors after preprocessing", + errors.len()); for error in errors { + debug!("report_region_errors: error = {:?}", error); - match error.clone() { - ConcreteFailure(origin, sub, sup) => { - self.report_concrete_failure(origin, sub, sup).emit(); - } + // If ConcreteFailure does not have an anonymous region + if !self.report_named_anon_conflict(&error){ - GenericBoundFailure(kind, param_ty, sub) => { - self.report_generic_bound_failure(kind, param_ty, sub); - } + match error.clone() { + + ConcreteFailure(origin, sub, sup) => { + + self.report_concrete_failure(origin, sub, sup).emit(); + } - SubSupConflict(var_origin, + GenericBoundFailure(kind, param_ty, sub) => { + self.report_generic_bound_failure(kind, param_ty, sub); + } + + SubSupConflict(var_origin, sub_origin, sub_r, sup_origin, sup_r) => { - self.report_sub_sup_conflict(var_origin, + self.report_sub_sup_conflict(var_origin, sub_origin, sub_r, sup_origin, sup_r); - } + } + } } } } diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs new file mode 100644 index 0000000000000..d48cb398ec282 --- /dev/null +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -0,0 +1,115 @@ +// Copyright 2012-2013 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. + +//! Error Reporting for Anonymous Region Lifetime Errors. +use hir; +use infer::InferCtxt; +use ty::{self, Region}; +use infer::region_inference::RegionResolutionError::*; +use infer::region_inference::RegionResolutionError; + +impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { + // This method walks the Type of the function body arguments using + // `fold_regions()` function and returns the + // &hir::Arg of the function argument corresponding to the anonymous + // region and the Ty corresponding to the named region. + // Currently only the case where the function declaration consists of + // one named region and one anonymous region is handled. + // Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32` + // Here, the `y` and the `Ty` of `y` is returned after being substituted + // by that of the named region. + pub fn find_arg_with_anonymous_region(&self, + anon_region: Region<'tcx>, + named_region: Region<'tcx>) + -> Option<(&hir::Arg, ty::Ty<'tcx>)> { + + match *anon_region { + ty::ReFree(ref free_region) => { + + let id = free_region.scope; + let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); + let body_id = self.tcx.hir.maybe_body_owned_by(node_id).unwrap(); + + let body = self.tcx.hir.body(body_id); + body.arguments + .iter() + .filter_map(|arg| if let Some(tables) = self.in_progress_tables { + let ty = tables.borrow().node_id_to_type(arg.id); + let mut found_anon_region = false; + let new_arg_ty = self.tcx + .fold_regions(&ty, + &mut false, + |r, _| if *r == *anon_region { + found_anon_region = true; + named_region + } else { + r + }); + if found_anon_region { + return Some((arg, new_arg_ty)); + } else { + None + } + } else { + None + }) + .next() + } + _ => None, + } + + } + + // 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(..)` + pub fn 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 + }; + + let (named, (var, new_ty)) = + if self.is_named_region(sub) && self.is_anonymous_region(sup) { + (sub, self.find_arg_with_anonymous_region(sup, sub).unwrap()) + } else if self.is_named_region(sup) && self.is_anonymous_region(sub) { + (sup, self.find_arg_with_anonymous_region(sub, sup).unwrap()) + } else { + return false; // inapplicable + }; + + if let Some(simple_name) = var.pat.simple_name() { + struct_span_err!(self.tcx.sess, + var.pat.span, + E0611, + "explicit lifetime required in the type of `{}`", + simple_name) + .span_label(var.pat.span, + format!("consider changing the type of `{}` to `{}`", + simple_name, + new_ty)) + .span_label(span, format!("lifetime `{}` required", named)) + .emit(); + + } else { + struct_span_err!(self.tcx.sess, + var.pat.span, + E0611, + "explicit lifetime required in parameter type") + .span_label(var.pat.span, + format!("consider changing type to `{}`", new_ty)) + .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 new file mode 100644 index 0000000000000..ea0c706d8aa71 --- /dev/null +++ b/src/librustc/infer/error_reporting/util.rs @@ -0,0 +1,56 @@ +// Copyright 2012-2013 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. + +//! Helper for error reporting code for named_anon_conflict + +use ty::{self, Region}; +use infer::InferCtxt; +use hir::map as hir_map; + +impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { + // This method returns whether the given Region is Named + pub fn is_named_region(&self, region: Region<'tcx>) -> bool { + + match *region { + ty::ReFree(ref free_region) => { + match free_region.bound_region { + ty::BrNamed(..) => true, + _ => false, + } + } + _ => false, + } + } + + // This method returns whether the given Region is Anonymous + pub fn is_anonymous_region(&self, region: Region<'tcx>) -> bool { + + match *region { + ty::ReFree(ref free_region) => { + match free_region.bound_region { + ty::BrAnon(..) => { + let id = free_region.scope; + let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); + match self.tcx.hir.find(node_id) { + Some(hir_map::NodeItem(..)) | + Some(hir_map::NodeImplItem(..)) | + Some(hir_map::NodeTraitItem(..)) => { /* proceed ahead */ } + _ => return false, // inapplicable + // we target only top-level functions + } + return true; + } + _ => false, + } + } + _ => false, + } + } +} diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index d5020b12ee00e..07de44c92948d 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -38,7 +38,6 @@ use errors::DiagnosticBuilder; use syntax_pos::{self, Span, DUMMY_SP}; use util::nodemap::FxHashMap; use arena::DroplessArena; - use self::combine::CombineFields; use self::higher_ranked::HrMatchResult; use self::region_inference::{RegionVarBindings, RegionSnapshot}; @@ -1077,6 +1076,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { region_map, free_regions); let errors = self.region_vars.resolve_regions(®ion_rels); + if !self.is_tainted_by_errors() { // As a heuristic, just skip reporting region errors // altogether if other errors have been reported while diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.rs new file mode 100644 index 0000000000000..a1716c4e79792 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.rs @@ -0,0 +1,15 @@ +// Copyright 2016 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. + +fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { + if x > y { x } else { y } +} + +fn main() { } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr new file mode 100644 index 0000000000000..a04a9461eb490 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr @@ -0,0 +1,10 @@ +error[E0611]: explicit lifetime required in the type of `x` + --> $DIR/ex1-return-one-existing-name-if-else-2.rs:11:12 + | +11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { + | ^ consider changing the type of `x` to `&'a i32` +12 | if x > y { x } else { y } + | - lifetime `'a` required + +error: aborting due to previous error(s) + diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.rs new file mode 100644 index 0000000000000..7bd32d8761705 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.rs @@ -0,0 +1,15 @@ +// Copyright 2016 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. + +fn foo<'a>((x, y): (&'a i32, &i32)) -> &'a i32 { + if x > y { x } else { y } +} + +fn main () { } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr new file mode 100644 index 0000000000000..143021cbbdd4d --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr @@ -0,0 +1,10 @@ +error[E0611]: explicit lifetime required in parameter type + --> $DIR/ex1-return-one-existing-name-if-else-3.rs:11:12 + | +11 | fn foo<'a>((x, y): (&'a i32, &i32)) -> &'a i32 { + | ^^^^^^ consider changing type to `(&'a i32, &'a i32)` +12 | if x > y { x } else { y } + | - lifetime `'a` required + +error: aborting due to previous error(s) + diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs new file mode 100644 index 0000000000000..faf4fe547bf00 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs @@ -0,0 +1,24 @@ +// Copyright 2016 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. + +fn invoke<'a, F>(x: &'a i32, f: F) -> &'a i32 +where F: FnOnce(&'a i32, &i32) -> &'a i32 +{ + let y = 22; + f(x, &y) +} + +fn foo<'a>(x: &'a i32) { + invoke(&x, |a, b| if a > b { a } else { b }); +} + +fn main() { +} + diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr new file mode 100644 index 0000000000000..c96ff9bc11481 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr @@ -0,0 +1,29 @@ +error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements + --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 + | +19 | invoke(&x, |a, b| if a > b { a } else { b }); + | ^^^^^^ + | +note: first, the lifetime cannot outlive the anonymous lifetime #2 defined on the body at 19:16... + --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:16 + | +19 | invoke(&x, |a, b| if a > b { a } else { b }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: ...so that reference does not outlive borrowed content + --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:45 + | +19 | invoke(&x, |a, b| if a > b { a } else { b }); + | ^ +note: but, the lifetime must be valid for the call at 19:5... + --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 + | +19 | invoke(&x, |a, b| if a > b { a } else { b }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: ...so that argument is valid for the call + --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:12 + | +19 | invoke(&x, |a, b| if a > b { a } else { b }); + | ^^ + +error: aborting due to previous error(s) + diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr index 0ab24b0b3e6c4..84f166dfa3023 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr @@ -1,23 +1,10 @@ -error[E0312]: lifetime of reference outlives lifetime of borrowed content... - --> $DIR/ex1-return-one-existing-name-if-else.rs:12:27 +error[E0611]: explicit lifetime required in the type of `y` + --> $DIR/ex1-return-one-existing-name-if-else.rs:11:24 | +11 | fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { + | ^ consider changing the type of `y` to `&'a i32` 12 | if x > y { x } else { y } - | ^ - | -note: ...the reference is valid for the lifetime 'a as defined on the function body at 11:1... - --> $DIR/ex1-return-one-existing-name-if-else.rs:11:1 - | -11 | / fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { -12 | | if x > y { x } else { y } -13 | | } - | |_^ -note: ...but the borrowed content is only valid for the anonymous lifetime #1 defined on the function body at 11:1 - --> $DIR/ex1-return-one-existing-name-if-else.rs:11:1 - | -11 | / fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { -12 | | if x > y { x } else { y } -13 | | } - | |_^ + | - lifetime `'a` required error: aborting due to previous error(s) diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.rs b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.rs new file mode 100644 index 0000000000000..dd34e1aa6d9d2 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.rs @@ -0,0 +1,19 @@ +// Copyright 2016 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. + +struct Ref<'a, T: 'a> { + data: &'a T +} + +fn foo<'a>(x: Ref, y: &mut Vec>) { + y.push(x); +} + +fn main() { } diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr new file mode 100644 index 0000000000000..61e97dc2bcd80 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr @@ -0,0 +1,10 @@ +error[E0611]: explicit lifetime required in the type of `x` + --> $DIR/ex2a-push-one-existing-name-2.rs:15:12 + | +15 | fn foo<'a>(x: Ref, y: &mut Vec>) { + | ^ consider changing the type of `x` to `Ref<'a, i32>` +16 | y.push(x); + | - lifetime `'a` required + +error: aborting due to previous error(s) + diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr index 7d0947b364e03..51d86a1f964ca 100644 --- a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr @@ -1,25 +1,10 @@ -error[E0308]: mismatched types - --> $DIR/ex2a-push-one-existing-name.rs:16:12 +error[E0611]: explicit lifetime required in the type of `y` + --> $DIR/ex2a-push-one-existing-name.rs:15:39 | +15 | fn foo<'a>(x: &mut Vec>, y: Ref) { + | ^ consider changing the type of `y` to `Ref<'a, i32>` 16 | x.push(y); - | ^ lifetime mismatch - | - = note: expected type `Ref<'a, _>` - found type `Ref<'_, _>` -note: the anonymous lifetime #2 defined on the function body at 15:1... - --> $DIR/ex2a-push-one-existing-name.rs:15:1 - | -15 | / fn foo<'a>(x: &mut Vec>, y: Ref) { -16 | | x.push(y); -17 | | } - | |_^ -note: ...does not necessarily outlive the lifetime 'a as defined on the function body at 15:1 - --> $DIR/ex2a-push-one-existing-name.rs:15:1 - | -15 | / fn foo<'a>(x: &mut Vec>, y: Ref) { -16 | | x.push(y); -17 | | } - | |_^ + | - lifetime `'a` required error: aborting due to previous error(s) From 5df7a2c86334e89abf813b786bd03375bf62e770 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 15 Jun 2017 03:50:02 -0700 Subject: [PATCH 02/17] Adding new ui test for trait impl --- src/librustc/infer/error_reporting/util.rs | 1 - ...rn-one-existing-name-if-else-using-impl.rs | 27 +++++++++++++++++++ ...ne-existing-name-if-else-using-impl.stderr | 27 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr diff --git a/src/librustc/infer/error_reporting/util.rs b/src/librustc/infer/error_reporting/util.rs index ea0c706d8aa71..38bb3e93d0020 100644 --- a/src/librustc/infer/error_reporting/util.rs +++ b/src/librustc/infer/error_reporting/util.rs @@ -40,7 +40,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); match self.tcx.hir.find(node_id) { Some(hir_map::NodeItem(..)) | - Some(hir_map::NodeImplItem(..)) | Some(hir_map::NodeTraitItem(..)) => { /* proceed ahead */ } _ => return false, // inapplicable // we target only top-level functions diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.rs new file mode 100644 index 0000000000000..36d956a39966f --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.rs @@ -0,0 +1,27 @@ +// Copyright 2016 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. + +trait Foo { + + fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32; + +} + +impl Foo for () { + + fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { + + if x > y { x } else { y } + + } + +} + +fn main() {} diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr new file mode 100644 index 0000000000000..15ecca618052e --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl.stderr @@ -0,0 +1,27 @@ +error[E0312]: lifetime of reference outlives lifetime of borrowed content... + --> $DIR/ex1-return-one-existing-name-if-else-using-impl.rs:21:20 + | +21 | if x > y { x } else { y } + | ^ + | +note: ...the reference is valid for the lifetime 'a as defined on the method body at 19:5... + --> $DIR/ex1-return-one-existing-name-if-else-using-impl.rs:19:5 + | +19 | / fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { +20 | | +21 | | if x > y { x } else { y } +22 | | +23 | | } + | |_____^ +note: ...but the borrowed content is only valid for the anonymous lifetime #1 defined on the method body at 19:5 + --> $DIR/ex1-return-one-existing-name-if-else-using-impl.rs:19:5 + | +19 | / fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { +20 | | +21 | | if x > y { x } else { y } +22 | | +23 | | } + | |_____^ + +error: aborting due to previous error(s) + From 4bed5f0094b632ec2a10d73074fcd02e289e20cc Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 15 Jun 2017 15:01:44 -0400 Subject: [PATCH 03/17] update reference for test --- ...turn-one-existing-name-if-else-using-closure.stderr | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr index c96ff9bc11481..20104afae516a 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr @@ -14,16 +14,16 @@ note: ...so that reference does not outlive borrowed content | 19 | invoke(&x, |a, b| if a > b { a } else { b }); | ^ -note: but, the lifetime must be valid for the call at 19:5... +note: but, the lifetime must be valid for the expression at 19:5... --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 | 19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -note: ...so that argument is valid for the call - --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:12 + | ^^^^^^ +note: ...so that a type/lifetime parameter is in scope here + --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 | 19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^^ + | ^^^^^^ error: aborting due to previous error(s) From ae92bd095c16c3aa14b986b089a1ded8df4c8369 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 15 Jun 2017 13:53:49 -0700 Subject: [PATCH 04/17] Interchange ^ and - --- src/librustc/infer/error_reporting/named_anon_conflict.rs | 4 ++-- .../ex1-return-one-existing-name-if-else-2.stderr | 6 +++--- .../ex1-return-one-existing-name-if-else-3.stderr | 6 +++--- .../ex1-return-one-existing-name-if-else.stderr | 6 +++--- .../ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr | 6 +++--- .../ui/lifetime-errors/ex2a-push-one-existing-name.stderr | 6 +++--- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index d48cb398ec282..77af22169102d 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -88,7 +88,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { if let Some(simple_name) = var.pat.simple_name() { struct_span_err!(self.tcx.sess, - var.pat.span, + span, E0611, "explicit lifetime required in the type of `{}`", simple_name) @@ -101,7 +101,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } else { struct_span_err!(self.tcx.sess, - var.pat.span, + span, E0611, "explicit lifetime required in parameter type") .span_label(var.pat.span, diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr index a04a9461eb490..ada7af8c1e46f 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr @@ -1,10 +1,10 @@ error[E0611]: explicit lifetime required in the type of `x` - --> $DIR/ex1-return-one-existing-name-if-else-2.rs:11:12 + --> $DIR/ex1-return-one-existing-name-if-else-2.rs:12:16 | 11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { - | ^ consider changing the type of `x` to `&'a i32` + | - consider changing the type of `x` to `&'a i32` 12 | if x > y { x } else { y } - | - lifetime `'a` required + | ^ lifetime `'a` required error: aborting due to previous error(s) diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr index 143021cbbdd4d..58aab71139440 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr @@ -1,10 +1,10 @@ error[E0611]: explicit lifetime required in parameter type - --> $DIR/ex1-return-one-existing-name-if-else-3.rs:11:12 + --> $DIR/ex1-return-one-existing-name-if-else-3.rs:12:27 | 11 | fn foo<'a>((x, y): (&'a i32, &i32)) -> &'a i32 { - | ^^^^^^ consider changing type to `(&'a i32, &'a i32)` + | ------ consider changing type to `(&'a i32, &'a i32)` 12 | if x > y { x } else { y } - | - lifetime `'a` required + | ^ lifetime `'a` required error: aborting due to previous error(s) diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr index 84f166dfa3023..837fa141bf1c2 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr @@ -1,10 +1,10 @@ error[E0611]: explicit lifetime required in the type of `y` - --> $DIR/ex1-return-one-existing-name-if-else.rs:11:24 + --> $DIR/ex1-return-one-existing-name-if-else.rs:12:27 | 11 | fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { - | ^ consider changing the type of `y` to `&'a i32` + | - consider changing the type of `y` to `&'a i32` 12 | if x > y { x } else { y } - | - lifetime `'a` required + | ^ lifetime `'a` required error: aborting due to previous error(s) diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr index 61e97dc2bcd80..a16dac672aeda 100644 --- a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr @@ -1,10 +1,10 @@ error[E0611]: explicit lifetime required in the type of `x` - --> $DIR/ex2a-push-one-existing-name-2.rs:15:12 + --> $DIR/ex2a-push-one-existing-name-2.rs:16:12 | 15 | fn foo<'a>(x: Ref, y: &mut Vec>) { - | ^ consider changing the type of `x` to `Ref<'a, i32>` + | - consider changing the type of `x` to `Ref<'a, i32>` 16 | y.push(x); - | - lifetime `'a` required + | ^ lifetime `'a` required error: aborting due to previous error(s) diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr index 51d86a1f964ca..537090aa67de7 100644 --- a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr @@ -1,10 +1,10 @@ error[E0611]: explicit lifetime required in the type of `y` - --> $DIR/ex2a-push-one-existing-name.rs:15:39 + --> $DIR/ex2a-push-one-existing-name.rs:16:12 | 15 | fn foo<'a>(x: &mut Vec>, y: Ref) { - | ^ consider changing the type of `y` to `Ref<'a, i32>` + | - consider changing the type of `y` to `Ref<'a, i32>` 16 | x.push(y); - | - lifetime `'a` required + | ^ lifetime `'a` required error: aborting due to previous error(s) From 8fb6f74e57f6c75113074b56f48b16992c5ce1e1 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Sat, 17 Jun 2017 16:27:07 -0700 Subject: [PATCH 05/17] Enabling E0611 for inherent functions --- src/librustc/infer/error_reporting/mod.rs | 14 +++- .../error_reporting/named_anon_conflict.rs | 77 +++++++++++++++---- src/librustc/infer/error_reporting/util.rs | 25 ------ ...-one-existing-name-if-else-using-impl-2.rs | 18 +++++ ...-existing-name-if-else-using-impl-2.stderr | 10 +++ ...-one-existing-name-if-else-using-impl-3.rs | 24 ++++++ ...-existing-name-if-else-using-impl-3.stderr | 11 +++ 7 files changed, 137 insertions(+), 42 deletions(-) create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 1bc01ab858c94..b3f7f2d376445 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -67,7 +67,7 @@ use hir::def_id::DefId; use middle::region; use traits::{ObligationCause, ObligationCauseCode}; use ty::{self, TyCtxt, TypeFoldable}; -use ty::{Region, Issue32330 }; +use ty::{Region, Issue32330}; use ty::error::TypeError; use syntax::ast::DUMMY_NODE_ID; use syntax_pos::{Pos, Span}; @@ -272,11 +272,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { for error in errors { debug!("report_region_errors: error = {:?}", error); - // If ConcreteFailure does not have an anonymous region - if !self.report_named_anon_conflict(&error){ + + if !self.try_report_named_anon_conflict(&error){ match error.clone() { - + // These errors could indicate all manner of different + // problems with many different solutions. Rather + // than generate a "one size fits all" error, what we + // attempt to do is go through a number of specific + // scenarios and try to find the best way to present + // the error. If all of these fails, we fall back to a rather + // general bit of code that displays the error information ConcreteFailure(origin, sub, sup) => { self.report_concrete_failure(origin, sub, sup).emit(); diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index 77af22169102d..8af9381107b8d 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -14,6 +14,7 @@ use infer::InferCtxt; use ty::{self, Region}; use infer::region_inference::RegionResolutionError::*; use infer::region_inference::RegionResolutionError; +use hir::map as hir_map; impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // This method walks the Type of the function body arguments using @@ -23,12 +24,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // Currently only the case where the function declaration consists of // one named region and one anonymous region is handled. // Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32` - // Here, the `y` and the `Ty` of `y` is returned after being substituted - // by that of the named region. - pub fn find_arg_with_anonymous_region(&self, - anon_region: Region<'tcx>, - named_region: Region<'tcx>) - -> Option<(&hir::Arg, ty::Ty<'tcx>)> { + // Here, we would return the hir::Arg for y, and we return the type &'a + // i32, which is the type of y but with the anonymous region replaced + // with 'a. + fn find_arg_with_anonymous_region(&self, + anon_region: Region<'tcx>, + named_region: Region<'tcx>) + -> Option<(&hir::Arg, ty::Ty<'tcx>)> { match *anon_region { ty::ReFree(ref free_region) => { @@ -70,29 +72,35 @@ 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(..)` - pub fn report_named_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool { + 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 }; - let (named, (var, new_ty)) = - if self.is_named_region(sub) && self.is_anonymous_region(sup) { + // Determine whether the sub and sup consist of one named region ('a) + // and one anonymous (elided) region. If so, find the parameter arg + // where the anonymous region appears (there must always be one; we + // only introduced anonymous regions in parameters) as well as a + // version new_ty of its type where the anonymous region is replaced + // with the named one. + let (named, (arg, new_ty)) = + if self.is_named_region(sub) && self.is_suitable_anonymous_region(sup) { (sub, self.find_arg_with_anonymous_region(sup, sub).unwrap()) - } else if self.is_named_region(sup) && self.is_anonymous_region(sub) { + } else if self.is_named_region(sup) && self.is_suitable_anonymous_region(sub) { (sup, self.find_arg_with_anonymous_region(sub, sup).unwrap()) } else { return false; // inapplicable }; - if let Some(simple_name) = var.pat.simple_name() { + if let Some(simple_name) = arg.pat.simple_name() { struct_span_err!(self.tcx.sess, span, E0611, "explicit lifetime required in the type of `{}`", simple_name) - .span_label(var.pat.span, + .span_label(arg.pat.span, format!("consider changing the type of `{}` to `{}`", simple_name, new_ty)) @@ -104,7 +112,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { span, E0611, "explicit lifetime required in parameter type") - .span_label(var.pat.span, + .span_label(arg.pat.span, format!("consider changing type to `{}`", new_ty)) .span_label(span, format!("lifetime `{}` required", named)) .emit(); @@ -112,4 +120,47 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { return true; } + + // This method returns whether the given Region is Anonymous + pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> bool { + + match *region { + ty::ReFree(ref free_region) => { + match free_region.bound_region { + ty::BrAnon(..) => { + let anonymous_region_binding_scope = free_region.scope; + let node_id = self.tcx + .hir + .as_local_node_id(anonymous_region_binding_scope) + .unwrap(); + match self.tcx.hir.find(node_id) { + Some(hir_map::NodeItem(..)) | + Some(hir_map::NodeTraitItem(..)) => { + // proceed ahead // + } + Some(hir_map::NodeImplItem(..)) => { + if self.tcx.impl_trait_ref(self.tcx. +associated_item(anonymous_region_binding_scope).container.id()).is_some() { + // For now, we do not try to target impls of traits. This is + // because this message is going to suggest that the user + // change the fn signature, but they may not be free to do so, + // since the signature must match the trait. + // + // FIXME(#42706) -- in some cases, we could do better here. + return false;//None; + } + else{ } + + } + _ => return false, // inapplicable + // we target only top-level functions + } + return true; + } + _ => false, + } + } + _ => false, + } + } } diff --git a/src/librustc/infer/error_reporting/util.rs b/src/librustc/infer/error_reporting/util.rs index 38bb3e93d0020..66c351b49ac68 100644 --- a/src/librustc/infer/error_reporting/util.rs +++ b/src/librustc/infer/error_reporting/util.rs @@ -12,7 +12,6 @@ use ty::{self, Region}; use infer::InferCtxt; -use hir::map as hir_map; impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // This method returns whether the given Region is Named @@ -28,28 +27,4 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { _ => false, } } - - // This method returns whether the given Region is Anonymous - pub fn is_anonymous_region(&self, region: Region<'tcx>) -> bool { - - match *region { - ty::ReFree(ref free_region) => { - match free_region.bound_region { - ty::BrAnon(..) => { - let id = free_region.scope; - let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); - match self.tcx.hir.find(node_id) { - Some(hir_map::NodeItem(..)) | - Some(hir_map::NodeTraitItem(..)) => { /* proceed ahead */ } - _ => return false, // inapplicable - // we target only top-level functions - } - return true; - } - _ => false, - } - } - _ => false, - } - } } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.rs new file mode 100644 index 0000000000000..8849f7084b3cd --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.rs @@ -0,0 +1,18 @@ +// Copyright 2016 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. + +trait Foo { + +fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { + if x > y { x } else { y } + } +} + +fn main() { } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr new file mode 100644 index 0000000000000..ec787eb749c01 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr @@ -0,0 +1,10 @@ +error[E0611]: explicit lifetime required in the type of `x` + --> $DIR/ex1-return-one-existing-name-if-else-using-impl-2.rs:14:15 + | +13 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { + | - consider changing the type of `x` to `&'a i32` +14 | if x > y { x } else { y } + | ^ lifetime `'a` required + +error: aborting due to previous error(s) + diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs new file mode 100644 index 0000000000000..60f794279a524 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs @@ -0,0 +1,24 @@ +// Copyright 2016 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. + +struct Foo { + field: i32 +} + +impl Foo { + fn foo<'a>(&'a self, x: &i32) -> &i32 { + + if true { &self.field } else { x } + + } + +} + +fn main() { } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr new file mode 100644 index 0000000000000..cedceb559d552 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr @@ -0,0 +1,11 @@ +error[E0611]: explicit lifetime required in the type of `x` + --> $DIR/ex1-return-one-existing-name-if-else-using-impl-3.rs:18:36 + | +16 | fn foo<'a>(&'a self, x: &i32) -> &i32 { + | - consider changing the type of `x` to `&'a i32` +17 | +18 | if true { &self.field } else { x } + | ^ lifetime `'a` required + +error: aborting due to previous error(s) + From 2d99ffd11b28357ed989a9641ed94ea384659d51 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Tue, 20 Jun 2017 06:42:11 -0700 Subject: [PATCH 06/17] track anonymous regions in return types, fix tidy errors --- src/librustc/infer/error_reporting/mod.rs | 2 +- .../error_reporting/named_anon_conflict.rs | 53 +++++++++++++------ ...-one-existing-name-if-else-using-impl-3.rs | 2 +- ...n-one-existing-name-return-type-is-anon.rs | 24 +++++++++ ...e-existing-name-return-type-is-anon.stderr | 27 ++++++++++ 5 files changed, 91 insertions(+), 17 deletions(-) create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index b3f7f2d376445..9e2d922b932d1 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -272,7 +272,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { for error in errors { debug!("report_region_errors: error = {:?}", error); - + if !self.try_report_named_anon_conflict(&error){ match error.clone() { diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index 8af9381107b8d..edb7887b504b1 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -15,6 +15,7 @@ use ty::{self, Region}; use infer::region_inference::RegionResolutionError::*; use infer::region_inference::RegionResolutionError; use hir::map as hir_map; +use hir::def_id::DefId; impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // This method walks the Type of the function body arguments using @@ -24,13 +25,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // Currently only the case where the function declaration consists of // one named region and one anonymous region is handled. // Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32` - // Here, we would return the hir::Arg for y, and we return the type &'a + // Here, we would return the hir::Arg for y, we return the type &'a // i32, which is the type of y but with the anonymous region replaced - // with 'a. + // with 'a and also the corresponding bound region. fn find_arg_with_anonymous_region(&self, anon_region: Region<'tcx>, named_region: Region<'tcx>) - -> Option<(&hir::Arg, ty::Ty<'tcx>)> { + -> Option<(&hir::Arg, ty::Ty<'tcx>, ty::BoundRegion)> { match *anon_region { ty::ReFree(ref free_region) => { @@ -55,7 +56,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { r }); if found_anon_region { - return Some((arg, new_arg_ty)); + return Some((arg, new_arg_ty, free_region.bound_region)); } else { None } @@ -85,15 +86,36 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // only introduced anonymous regions in parameters) as well as a // version new_ty of its type where the anonymous region is replaced // with the named one. - let (named, (arg, new_ty)) = - if self.is_named_region(sub) && self.is_suitable_anonymous_region(sup) { - (sub, self.find_arg_with_anonymous_region(sup, sub).unwrap()) - } else if self.is_named_region(sup) && self.is_suitable_anonymous_region(sub) { - (sup, self.find_arg_with_anonymous_region(sub, sup).unwrap()) + let (named, (arg, new_ty, br), scope_def_id) = + if self.is_named_region(sub) && self.is_suitable_anonymous_region(sup).is_some() { + (sub, + self.find_arg_with_anonymous_region(sup, sub).unwrap(), + self.is_suitable_anonymous_region(sup).unwrap()) + } else if self.is_named_region(sup) && + self.is_suitable_anonymous_region(sub).is_some() { + (sup, + self.find_arg_with_anonymous_region(sub, sup).unwrap(), + self.is_suitable_anonymous_region(sub).unwrap()) } else { return false; // inapplicable }; + // Here, we check for the case where the anonymous region + // is in the return type. + // FIXME(#42703) - Need to handle certain cases here. + let ret_ty = self.tcx.type_of(scope_def_id); + match ret_ty.sty { + ty::TyFnDef(_, _, sig) => { + let late_bound_regions = self.tcx + .collect_referenced_late_bound_regions(&sig.output()); + if late_bound_regions.iter().any(|r| *r == br) { + return false; + } else { + } + } + _ => {} + } + if let Some(simple_name) = arg.pat.simple_name() { struct_span_err!(self.tcx.sess, span, @@ -122,7 +144,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } // This method returns whether the given Region is Anonymous - pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> bool { + // and returns the DefId corresponding to the region. + pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> Option { match *region { ty::ReFree(ref free_region) => { @@ -147,20 +170,20 @@ associated_item(anonymous_region_binding_scope).container.id()).is_some() { // since the signature must match the trait. // // FIXME(#42706) -- in some cases, we could do better here. - return false;//None; + return None; } else{ } } - _ => return false, // inapplicable + _ => return None, // inapplicable // we target only top-level functions } - return true; + return Some(anonymous_region_binding_scope); } - _ => false, + _ => None, } } - _ => false, + _ => None, } } } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs index 60f794279a524..362290ff3fa7d 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.rs @@ -14,7 +14,7 @@ struct Foo { impl Foo { fn foo<'a>(&'a self, x: &i32) -> &i32 { - + if true { &self.field } else { x } } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.rs new file mode 100644 index 0000000000000..96b733be9b4eb --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.rs @@ -0,0 +1,24 @@ +// Copyright 2016 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. + +struct Foo { + field: i32 +} + +impl Foo { + fn foo<'a>(&self, x: &'a i32) -> &i32 { + + x + + } + +} + +fn main() { } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr new file mode 100644 index 0000000000000..e32de589d2870 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr @@ -0,0 +1,27 @@ +error[E0312]: lifetime of reference outlives lifetime of borrowed content... + --> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:18:5 + | +18 | x + | ^ + | +note: ...the reference is valid for the anonymous lifetime #1 defined on the method body at 16:3... + --> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:16:3 + | +16 | / fn foo<'a>(&self, x: &'a i32) -> &i32 { +17 | | +18 | | x +19 | | +20 | | } + | |___^ +note: ...but the borrowed content is only valid for the lifetime 'a as defined on the method body at 16:3 + --> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:16:3 + | +16 | / fn foo<'a>(&self, x: &'a i32) -> &i32 { +17 | | +18 | | x +19 | | +20 | | } + | |___^ + +error: aborting due to previous error(s) + From a851e1e543c944b38472751d251593f3c593cc3a Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Wed, 21 Jun 2017 07:01:35 -0700 Subject: [PATCH 07/17] Adding changes to track anonymous region in self --- .../error_reporting/named_anon_conflict.rs | 60 ++++++++++++------- ...-existing-name-if-else-using-impl-3.stderr | 2 +- ...e-existing-name-return-type-is-anon.stderr | 4 +- ...1-return-one-existing-name-self-is-anon.rs | 23 +++++++ ...turn-one-existing-name-self-is-anon.stderr | 27 +++++++++ 5 files changed, 93 insertions(+), 23 deletions(-) create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.rs create mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index edb7887b504b1..a63ae186f61d8 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -27,11 +27,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // Consider the example `fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32` // Here, we would return the hir::Arg for y, we return the type &'a // i32, which is the type of y but with the anonymous region replaced - // with 'a and also the corresponding bound region. - fn find_arg_with_anonymous_region(&self, - anon_region: Region<'tcx>, - named_region: Region<'tcx>) - -> Option<(&hir::Arg, ty::Ty<'tcx>, ty::BoundRegion)> { + // with 'a, the corresponding bound region and is_first which is true if + // the hir::Arg is the first argument in the function declaration. + fn find_arg_with_anonymous_region + (&self, + anon_region: Region<'tcx>, + named_region: Region<'tcx>) + -> Option<(&hir::Arg, ty::Ty<'tcx>, ty::BoundRegion, bool)> { match *anon_region { ty::ReFree(ref free_region) => { @@ -39,7 +41,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let id = free_region.scope; let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); let body_id = self.tcx.hir.maybe_body_owned_by(node_id).unwrap(); - + let mut is_first = false; let body = self.tcx.hir.body(body_id); body.arguments .iter() @@ -56,7 +58,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { r }); if found_anon_region { - return Some((arg, new_arg_ty, free_region.bound_region)); + if body.arguments.iter().nth(0) == Some(&arg) { + is_first = true; + } + return Some((arg, + new_arg_ty, + free_region.bound_region, + is_first)); } else { None } @@ -86,19 +94,19 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // only introduced anonymous regions in parameters) as well as a // version new_ty of its type where the anonymous region is replaced // with the named one. - let (named, (arg, new_ty, br), scope_def_id) = - if self.is_named_region(sub) && self.is_suitable_anonymous_region(sup).is_some() { - (sub, - self.find_arg_with_anonymous_region(sup, sub).unwrap(), - self.is_suitable_anonymous_region(sup).unwrap()) - } else if self.is_named_region(sup) && - self.is_suitable_anonymous_region(sub).is_some() { - (sup, - self.find_arg_with_anonymous_region(sub, sup).unwrap(), - self.is_suitable_anonymous_region(sub).unwrap()) - } else { - return false; // inapplicable - }; + let (named, (arg, new_ty, br, is_first), scope_def_id) = if + self.is_named_region(sub) && self.is_suitable_anonymous_region(sup).is_some() { + (sub, + self.find_arg_with_anonymous_region(sup, sub).unwrap(), + self.is_suitable_anonymous_region(sup).unwrap()) + } else if + self.is_named_region(sup) && self.is_suitable_anonymous_region(sub).is_some() { + (sup, + self.find_arg_with_anonymous_region(sub, sup).unwrap(), + self.is_suitable_anonymous_region(sub).unwrap()) + } else { + return false; // inapplicable + }; // Here, we check for the case where the anonymous region // is in the return type. @@ -116,6 +124,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { _ => {} } + // Here we check for the case where anonymous region + // corresponds to self and if yes, we display E0312. + // FIXME(#42700) - Need to format self properly to + // enable E0611 for it. + if is_first && + self.tcx + .opt_associated_item(scope_def_id) + .map(|i| i.method_has_self_argument) + .unwrap_or(false) { + return false; + } + if let Some(simple_name) = arg.pat.simple_name() { struct_span_err!(self.tcx.sess, span, diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr index cedceb559d552..502871022ff6f 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr @@ -3,7 +3,7 @@ error[E0611]: explicit lifetime required in the type of `x` | 16 | fn foo<'a>(&'a self, x: &i32) -> &i32 { | - consider changing the type of `x` to `&'a i32` -17 | +17 | 18 | if true { &self.field } else { x } | ^ lifetime `'a` required diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr index e32de589d2870..471b3401827d8 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr @@ -8,7 +8,7 @@ note: ...the reference is valid for the anonymous lifetime #1 defined on the met --> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:16:3 | 16 | / fn foo<'a>(&self, x: &'a i32) -> &i32 { -17 | | +17 | | 18 | | x 19 | | 20 | | } @@ -17,7 +17,7 @@ note: ...but the borrowed content is only valid for the lifetime 'a as defined o --> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:16:3 | 16 | / fn foo<'a>(&self, x: &'a i32) -> &i32 { -17 | | +17 | | 18 | | x 19 | | 20 | | } diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.rs b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.rs new file mode 100644 index 0000000000000..a8ce60c47b6f5 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.rs @@ -0,0 +1,23 @@ +// Copyright 2016 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. + +struct Foo { + field: i32, +} + +impl Foo { + fn foo<'a>(&self, x: &'a Foo) -> &'a Foo { + + if true { x } else { self } + + } +} + +fn main() {} diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr new file mode 100644 index 0000000000000..46fc43eaf5756 --- /dev/null +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr @@ -0,0 +1,27 @@ +error[E0312]: lifetime of reference outlives lifetime of borrowed content... + --> $DIR/ex1-return-one-existing-name-self-is-anon.rs:18:30 + | +18 | if true { x } else { self } + | ^^^^ + | +note: ...the reference is valid for the lifetime 'a as defined on the method body at 16:5... + --> $DIR/ex1-return-one-existing-name-self-is-anon.rs:16:5 + | +16 | / fn foo<'a>(&self, x: &'a Foo) -> &'a Foo { +17 | | +18 | | if true { x } else { self } +19 | | +20 | | } + | |_____^ +note: ...but the borrowed content is only valid for the anonymous lifetime #1 defined on the method body at 16:5 + --> $DIR/ex1-return-one-existing-name-self-is-anon.rs:16:5 + | +16 | / fn foo<'a>(&self, x: &'a Foo) -> &'a Foo { +17 | | +18 | | if true { x } else { self } +19 | | +20 | | } + | |_____^ + +error: aborting due to previous error(s) + From 82f25b32ae568986de275d87fe355ec63307d8cb Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 22 Jun 2017 10:56:05 -0700 Subject: [PATCH 08/17] code review fixes --- .../error_reporting/named_anon_conflict.rs | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index a63ae186f61d8..4a1f4b418ae4a 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -43,39 +43,36 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let body_id = self.tcx.hir.maybe_body_owned_by(node_id).unwrap(); let mut is_first = false; let body = self.tcx.hir.body(body_id); - body.arguments - .iter() - .filter_map(|arg| if let Some(tables) = self.in_progress_tables { - let ty = tables.borrow().node_id_to_type(arg.id); - let mut found_anon_region = false; - let new_arg_ty = self.tcx - .fold_regions(&ty, - &mut false, - |r, _| if *r == *anon_region { - found_anon_region = true; - named_region - } else { - r - }); - if found_anon_region { - if body.arguments.iter().nth(0) == Some(&arg) { - is_first = true; - } - return Some((arg, - new_arg_ty, - free_region.bound_region, - is_first)); - } else { - None - } + if let Some(tables) = self.in_progress_tables { + body.arguments + .iter() + .filter_map(|arg| { + let ty = tables.borrow().node_id_to_type(arg.id); + let mut found_anon_region = false; + let new_arg_ty = self.tcx + .fold_regions(&ty, &mut false, |r, _| if *r == *anon_region { + found_anon_region = true; + named_region } else { - None - }) - .next() + r + }); + if found_anon_region { + if body.arguments.iter().nth(0) == Some(&arg) { + is_first = true; + } + Some((arg, new_arg_ty, free_region.bound_region, is_first)) + } else { + None + } + }) + .next() + } else { + None + } } _ => None, - } + } } // This method generates the error message for the case when From aebc4e007493c623aaf69ef04d1a649b74fd5bfb Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Mon, 26 Jun 2017 11:26:01 -0700 Subject: [PATCH 09/17] Changing the error code to E0621 --- src/librustc/diagnostics.rs | 31 ++++++++++++++----- .../error_reporting/named_anon_conflict.rs | 6 ++-- ...-return-one-existing-name-if-else-2.stderr | 2 +- ...-return-one-existing-name-if-else-3.stderr | 2 +- ...-existing-name-if-else-using-impl-2.stderr | 2 +- ...-existing-name-if-else-using-impl-3.stderr | 2 +- ...x1-return-one-existing-name-if-else.stderr | 2 +- .../ex2a-push-one-existing-name-2.stderr | 2 +- .../ex2a-push-one-existing-name.stderr | 2 +- 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 1e7f3f9aeb8b9..aa62cab7c3dcc 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -1946,11 +1946,19 @@ Maybe you just misspelled the lint name or the lint doesn't exist anymore. Either way, try to update/remove it in order to fix the error. "##, -E0611: r##" -Lifetime parameter is missing in one of the function argument. Erroneous -code example: - -```compile_fail,E0611 +E0621: r##" +This error code indicates a mismatch between the function signature (i.e., +the parameter types and the return type) and the function body. Most of +the time, this indicates that the function signature needs to be changed to +match the body, but it may be that the body needs to be changed to match +the signature. + +Specifically, one or more of the parameters contain borrowed data that +needs to have a named lifetime in order for the body to type-check. Most of +the time, this is because the borrowed data is being returned from the +function, as in this example: + +```compile_fail,E0621 fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { // explicit lifetime required // in the type of `y` if x > y { x } else { y } @@ -1959,15 +1967,24 @@ fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { // explicit lifetime required fn main () { } ``` -Please add the missing lifetime parameter to remove this error. Example: +Here, the function is returning data borrowed from either x or y, but the +'a annotation indicates that it is returning data only from x. We can make +the signature match the body by changing the type of y to &'a i32, like so: ``` fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 { if x > y { x } else { y } } -fn main() { +fn main () { } +``` +Alternatively, you could change the body not to return data from y: +``` +fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { + x } + +fn main () { } ``` "##, diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index 4a1f4b418ae4a..fb0bd901db4b7 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -124,7 +124,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // Here we check for the case where anonymous region // corresponds to self and if yes, we display E0312. // FIXME(#42700) - Need to format self properly to - // enable E0611 for it. + // enable E0621 for it. if is_first && self.tcx .opt_associated_item(scope_def_id) @@ -136,7 +136,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { if let Some(simple_name) = arg.pat.simple_name() { struct_span_err!(self.tcx.sess, span, - E0611, + E0621, "explicit lifetime required in the type of `{}`", simple_name) .span_label(arg.pat.span, @@ -149,7 +149,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } else { struct_span_err!(self.tcx.sess, span, - E0611, + E0621, "explicit lifetime required in parameter type") .span_label(arg.pat.span, format!("consider changing type to `{}`", new_ty)) diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr index ada7af8c1e46f..4d8c5e039af41 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in the type of `x` +error[E0621]: explicit lifetime required in the type of `x` --> $DIR/ex1-return-one-existing-name-if-else-2.rs:12:16 | 11 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr index 58aab71139440..07b276601f47c 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in parameter type +error[E0621]: explicit lifetime required in parameter type --> $DIR/ex1-return-one-existing-name-if-else-3.rs:12:27 | 11 | fn foo<'a>((x, y): (&'a i32, &i32)) -> &'a i32 { diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr index ec787eb749c01..2adf0cd762c59 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in the type of `x` +error[E0621]: explicit lifetime required in the type of `x` --> $DIR/ex1-return-one-existing-name-if-else-using-impl-2.rs:14:15 | 13 | fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 { diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr index 502871022ff6f..15825017d15c3 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in the type of `x` +error[E0621]: explicit lifetime required in the type of `x` --> $DIR/ex1-return-one-existing-name-if-else-using-impl-3.rs:18:36 | 16 | fn foo<'a>(&'a self, x: &i32) -> &i32 { diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr index 837fa141bf1c2..892a6dcd1e934 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr +++ b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in the type of `y` +error[E0621]: explicit lifetime required in the type of `y` --> $DIR/ex1-return-one-existing-name-if-else.rs:12:27 | 11 | fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr index a16dac672aeda..ea696c51d6218 100644 --- a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in the type of `x` +error[E0621]: explicit lifetime required in the type of `x` --> $DIR/ex2a-push-one-existing-name-2.rs:16:12 | 15 | fn foo<'a>(x: Ref, y: &mut Vec>) { diff --git a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr index 537090aa67de7..1630ae32ba6bf 100644 --- a/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr +++ b/src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr @@ -1,4 +1,4 @@ -error[E0611]: explicit lifetime required in the type of `y` +error[E0621]: explicit lifetime required in the type of `y` --> $DIR/ex2a-push-one-existing-name.rs:16:12 | 15 | fn foo<'a>(x: &mut Vec>, y: Ref) { From 95409016f8e84a47715526a89929cd22a2f25c16 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 27 Jun 2017 13:16:47 -0400 Subject: [PATCH 10/17] remove `fn main() { }` from extended errors --- src/librustc/diagnostics.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index aa62cab7c3dcc..035640b9710e2 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -1963,8 +1963,6 @@ fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { // explicit lifetime required // in the type of `y` if x > y { x } else { y } } - -fn main () { } ``` Here, the function is returning data borrowed from either x or y, but the @@ -1975,16 +1973,14 @@ the signature match the body by changing the type of y to &'a i32, like so: fn foo<'a>(x: &'a i32, y: &'a i32) -> &'a i32 { if x > y { x } else { y } } - -fn main () { } ``` + Alternatively, you could change the body not to return data from y: + ``` fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 { x } - -fn main () { } ``` "##, From e8b8f30373941f1d606d21c741bb136d81c3d082 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Tue, 27 Jun 2017 12:54:15 -0700 Subject: [PATCH 11/17] Code review fixes --- src/librustc/infer/error_reporting/mod.rs | 1 - .../error_reporting/named_anon_conflict.rs | 42 +++++++++---------- src/librustc/infer/error_reporting/util.rs | 30 ------------- src/librustc/ty/sty.rs | 14 +++++++ 4 files changed, 34 insertions(+), 53 deletions(-) delete mode 100644 src/librustc/infer/error_reporting/util.rs diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 9e2d922b932d1..82bbb4a1bf515 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -76,7 +76,6 @@ mod note; mod need_type_info; mod named_anon_conflict; -mod util; impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index fb0bd901db4b7..cdfb57c86f940 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -41,12 +41,12 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let id = free_region.scope; let node_id = self.tcx.hir.as_local_node_id(id).unwrap(); let body_id = self.tcx.hir.maybe_body_owned_by(node_id).unwrap(); - let mut is_first = false; let body = self.tcx.hir.body(body_id); if let Some(tables) = self.in_progress_tables { body.arguments .iter() - .filter_map(|arg| { + .enumerate() + .filter_map(|(index, arg)| { let ty = tables.borrow().node_id_to_type(arg.id); let mut found_anon_region = false; let new_arg_ty = self.tcx @@ -57,9 +57,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { r }); if found_anon_region { - if body.arguments.iter().nth(0) == Some(&arg) { - is_first = true; - } + let is_first = index == 0; Some((arg, new_arg_ty, free_region.bound_region, is_first)) } else { None @@ -91,19 +89,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // only introduced anonymous regions in parameters) as well as a // version new_ty of its type where the anonymous region is replaced // with the named one. - let (named, (arg, new_ty, br, is_first), scope_def_id) = if - self.is_named_region(sub) && self.is_suitable_anonymous_region(sup).is_some() { - (sub, - self.find_arg_with_anonymous_region(sup, sub).unwrap(), - self.is_suitable_anonymous_region(sup).unwrap()) - } else if - self.is_named_region(sup) && self.is_suitable_anonymous_region(sub).is_some() { - (sup, - self.find_arg_with_anonymous_region(sub, sup).unwrap(), - self.is_suitable_anonymous_region(sub).unwrap()) - } else { - return false; // inapplicable - }; + let (named, (arg, new_ty, br, is_first), scope_def_id) = + if sub.is_named_region() && self.is_suitable_anonymous_region(sup).is_some() { + (sub, + self.find_arg_with_anonymous_region(sup, sub).unwrap(), + self.is_suitable_anonymous_region(sup).unwrap()) + } else if sup.is_named_region() && self.is_suitable_anonymous_region(sub).is_some() { + (sup, + self.find_arg_with_anonymous_region(sub, sup).unwrap(), + self.is_suitable_anonymous_region(sub).unwrap()) + } else { + return false; // inapplicable + }; // Here, we check for the case where the anonymous region // is in the return type. @@ -179,8 +176,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // proceed ahead // } Some(hir_map::NodeImplItem(..)) => { - if self.tcx.impl_trait_ref(self.tcx. -associated_item(anonymous_region_binding_scope).container.id()).is_some() { + let container_id = self.tcx + .associated_item(anonymous_region_binding_scope) + .container + .id(); + if self.tcx.impl_trait_ref(container_id).is_some() { // For now, we do not try to target impls of traits. This is // because this message is going to suggest that the user // change the fn signature, but they may not be free to do so, @@ -189,8 +189,6 @@ associated_item(anonymous_region_binding_scope).container.id()).is_some() { // FIXME(#42706) -- in some cases, we could do better here. return None; } - else{ } - } _ => return None, // inapplicable // we target only top-level functions diff --git a/src/librustc/infer/error_reporting/util.rs b/src/librustc/infer/error_reporting/util.rs deleted file mode 100644 index 66c351b49ac68..0000000000000 --- a/src/librustc/infer/error_reporting/util.rs +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2012-2013 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. - -//! Helper for error reporting code for named_anon_conflict - -use ty::{self, Region}; -use infer::InferCtxt; - -impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { - // This method returns whether the given Region is Named - pub fn is_named_region(&self, region: Region<'tcx>) -> bool { - - match *region { - ty::ReFree(ref free_region) => { - match free_region.bound_region { - ty::BrNamed(..) => true, - _ => false, - } - } - _ => false, - } - } -} diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index ed3312d88a384..452775e9e1337 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -990,6 +990,20 @@ impl RegionKind { flags } + + // This method returns whether the given Region is Named + pub fn is_named_region(&self) -> bool { + + match *self { + ty::ReFree(ref free_region) => { + match free_region.bound_region { + ty::BrNamed(..) => true, + _ => false, + } + } + _ => false, + } + } } /// Type utilities From 5841021f071f7e6393e3ee510c4e02f44cb0fbee Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Wed, 28 Jun 2017 12:18:21 -0700 Subject: [PATCH 12/17] conflict fixes --- src/librustc/infer/error_reporting/named_anon_conflict.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index cdfb57c86f940..21f8a04be0a6a 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -107,7 +107,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // FIXME(#42703) - Need to handle certain cases here. let ret_ty = self.tcx.type_of(scope_def_id); match ret_ty.sty { - ty::TyFnDef(_, _, sig) => { + ty::TyFnDef(_, _) => { + let sig = ret_ty.fn_sig(self.tcx); let late_bound_regions = self.tcx .collect_referenced_late_bound_regions(&sig.output()); if late_bound_regions.iter().any(|r| *r == br) { From 5be4fa864af439cb18fe9bff4297e229f2879c73 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 29 Jun 2017 06:35:09 -0700 Subject: [PATCH 13/17] code fixes for error code use warning --- .../error_reporting/named_anon_conflict.rs | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/src/librustc/infer/error_reporting/named_anon_conflict.rs b/src/librustc/infer/error_reporting/named_anon_conflict.rs index 21f8a04be0a6a..ccbc5cdb862f9 100644 --- a/src/librustc/infer/error_reporting/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/named_anon_conflict.rs @@ -131,29 +131,23 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { return false; } - if let Some(simple_name) = arg.pat.simple_name() { - struct_span_err!(self.tcx.sess, - span, - E0621, - "explicit lifetime required in the type of `{}`", - simple_name) - .span_label(arg.pat.span, - format!("consider changing the type of `{}` to `{}`", - simple_name, - new_ty)) - .span_label(span, format!("lifetime `{}` required", named)) - .emit(); - + let (error_var, span_label_var) = if let Some(simple_name) = arg.pat.simple_name() { + (format!("the type of `{}`", simple_name), format!("the type of `{}`", simple_name)) } else { - struct_span_err!(self.tcx.sess, - span, - E0621, - "explicit lifetime required in parameter type") - .span_label(arg.pat.span, - format!("consider changing type to `{}`", new_ty)) - .span_label(span, format!("lifetime `{}` required", named)) - .emit(); - } + (format!("parameter type"), format!("type")) + }; + + + struct_span_err!(self.tcx.sess, + span, + E0621, + "explicit lifetime required in {}", + error_var) + .span_label(arg.pat.span, + format!("consider changing {} to `{}`", span_label_var, new_ty)) + .span_label(span, format!("lifetime `{}` required", named)) + .emit(); + return true; } From 4abcf28d2baaf53afe810e75ddfbfa8eff72c863 Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Thu, 29 Jun 2017 14:13:53 -0700 Subject: [PATCH 14/17] adding compile-fail test --- .../E0495.rs} | 7 ++--- ...existing-name-if-else-using-closure.stderr | 29 ------------------- 2 files changed, 3 insertions(+), 33 deletions(-) rename src/test/{ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs => compile-fail/E0495.rs} (84%) delete mode 100644 src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs b/src/test/compile-fail/E0495.rs similarity index 84% rename from src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs rename to src/test/compile-fail/E0495.rs index faf4fe547bf00..e47c4d7199c7a 100644 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.rs +++ b/src/test/compile-fail/E0495.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// FIXME - This test gives different results on different machines. fn invoke<'a, F>(x: &'a i32, f: F) -> &'a i32 where F: FnOnce(&'a i32, &i32) -> &'a i32 { @@ -15,10 +16,8 @@ where F: FnOnce(&'a i32, &i32) -> &'a i32 f(x, &y) } -fn foo<'a>(x: &'a i32) { +fn foo<'a>(x: &'a i32) { //~ ERROR E0495 invoke(&x, |a, b| if a > b { a } else { b }); } -fn main() { -} - +fn main() {} diff --git a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr b/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr deleted file mode 100644 index 20104afae516a..0000000000000 --- a/src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-closure.stderr +++ /dev/null @@ -1,29 +0,0 @@ -error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'a` due to conflicting requirements - --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 - | -19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^^^^^^ - | -note: first, the lifetime cannot outlive the anonymous lifetime #2 defined on the body at 19:16... - --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:16 - | -19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -note: ...so that reference does not outlive borrowed content - --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:45 - | -19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^ -note: but, the lifetime must be valid for the expression at 19:5... - --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 - | -19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^^^^^^ -note: ...so that a type/lifetime parameter is in scope here - --> $DIR/ex1-return-one-existing-name-if-else-using-closure.rs:19:5 - | -19 | invoke(&x, |a, b| if a > b { a } else { b }); - | ^^^^^^ - -error: aborting due to previous error(s) - From cb26a25d4b355e9836fc139bb71ad96d37e6c265 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 29 Jun 2017 18:02:31 -0400 Subject: [PATCH 15/17] tweak comments in E0495.rs --- src/test/compile-fail/E0495.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/test/compile-fail/E0495.rs b/src/test/compile-fail/E0495.rs index e47c4d7199c7a..55871a90f23c9 100644 --- a/src/test/compile-fail/E0495.rs +++ b/src/test/compile-fail/E0495.rs @@ -8,7 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME - This test gives different results on different machines. +// Test that we give the generic E0495 when one of the free regions is +// bound in a closure (rather than suggesting a change to the signature +// of the closure, which is not specified in `foo` but rather in `invoke`). + +// FIXME - This might be better as a UI test, but the finer details +// of the error seem to vary on different machines. fn invoke<'a, F>(x: &'a i32, f: F) -> &'a i32 where F: FnOnce(&'a i32, &i32) -> &'a i32 { From a43377773f3b38e4f521964a3b519f97f50fc3e4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 30 Jun 2017 05:11:28 -0400 Subject: [PATCH 16/17] move ERROR line --- src/test/compile-fail/E0495.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/compile-fail/E0495.rs b/src/test/compile-fail/E0495.rs index 55871a90f23c9..980461bedae55 100644 --- a/src/test/compile-fail/E0495.rs +++ b/src/test/compile-fail/E0495.rs @@ -21,8 +21,8 @@ where F: FnOnce(&'a i32, &i32) -> &'a i32 f(x, &y) } -fn foo<'a>(x: &'a i32) { //~ ERROR E0495 - invoke(&x, |a, b| if a > b { a } else { b }); +fn foo<'a>(x: &'a i32) { + invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495 } fn main() {} From 37a88f478dd80404b7b8c3890db96f5850ecd7bf Mon Sep 17 00:00:00 2001 From: gaurikholkar Date: Fri, 30 Jun 2017 02:33:33 -0700 Subject: [PATCH 17/17] rename compile-fail test --- .../{E0495.rs => E0621-does-not-trigger-for-closures.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/test/compile-fail/{E0495.rs => E0621-does-not-trigger-for-closures.rs} (100%) diff --git a/src/test/compile-fail/E0495.rs b/src/test/compile-fail/E0621-does-not-trigger-for-closures.rs similarity index 100% rename from src/test/compile-fail/E0495.rs rename to src/test/compile-fail/E0621-does-not-trigger-for-closures.rs