Skip to content

Commit

Permalink
Rollup merge of rust-lang#42669 - gaurikholkar:master, r=nikomatsakis
Browse files Browse the repository at this point in the history
Adding diagnostic code 0611 for lifetime errors with one named, one anonymous lifetime parameter

This is a fix for rust-lang#42517
Note that this only handles the above case for **function declarations** and **traits**.
`impl items` and `closures` will be handled in a later PR.
Example
```
fn foo<'a>(x: &i32, y: &'a i32) -> &'a i32 {
    if x > y { x } else { y }
}
```
now displays the following error message. ui tests have been added for the same.
```
error[E0611]: explicit lifetime required in the type of `x`
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
```
rust-lang#42516
r? @nikomatsakis
  • Loading branch information
GuillaumeGomez authored Jun 30, 2017
2 parents e72580c + 37a88f4 commit a47e48a
Show file tree
Hide file tree
Showing 24 changed files with 615 additions and 51 deletions.
38 changes: 38 additions & 0 deletions src/librustc/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1946,6 +1946,44 @@ 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.
"##,

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 }
}
```
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 }
}
```
Alternatively, you could change the body not to return data from y:
```
fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 {
x
}
```
"##,

}


Expand Down
44 changes: 30 additions & 14 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@ 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;


impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn note_and_explain_region(self,
Expand Down Expand Up @@ -255,34 +257,48 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}

impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
pub fn report_region_errors(&self,
errors: &Vec<RegionResolutionError<'tcx>>) {

pub fn report_region_errors(&self, errors: &Vec<RegionResolutionError<'tcx>>) {
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();
}

GenericBoundFailure(kind, param_ty, sub) => {
self.report_generic_bound_failure(kind, param_ty, sub);
}
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();
}

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);
}
}
}
}
}
}
Expand Down
199 changes: 199 additions & 0 deletions src/librustc/infer/error_reporting/named_anon_conflict.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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;
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
// `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, 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, 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) => {

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);
if let Some(tables) = self.in_progress_tables {
body.arguments
.iter()
.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
.fold_regions(&ty, &mut false, |r, _| if *r == *anon_region {
found_anon_region = true;
named_region
} else {
r
});
if found_anon_region {
let is_first = index == 0;
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
// 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
};

// 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, 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.
// FIXME(#42703) - Need to handle certain cases here.
let ret_ty = self.tcx.type_of(scope_def_id);
match ret_ty.sty {
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) {
return false;
} else {
}
}
_ => {}
}

// 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 E0621 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;
}

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 {
(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;

}

// This method returns whether the given Region is Anonymous
// and returns the DefId corresponding to the region.
pub fn is_suitable_anonymous_region(&self, region: Region<'tcx>) -> Option<DefId> {

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(..)) => {
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,
// since the signature must match the trait.
//
// FIXME(#42706) -- in some cases, we could do better here.
return None;
}
}
_ => return None, // inapplicable
// we target only top-level functions
}
return Some(anonymous_region_binding_scope);
}
_ => None,
}
}
_ => None,
}
}
}
2 changes: 1 addition & 1 deletion src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -1077,6 +1076,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
region_map,
free_regions);
let errors = self.region_vars.resolve_regions(&region_rels);

if !self.is_tainted_by_errors() {
// As a heuristic, just skip reporting region errors
// altogether if other errors have been reported while
Expand Down
14 changes: 14 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions src/test/compile-fail/E0621-does-not-trigger-for-closures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// 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
{
let y = 22;
f(x, &y)
}

fn foo<'a>(x: &'a i32) {
invoke(&x, |a, b| if a > b { a } else { b }); //~ ERROR E0495
}

fn main() {}
Loading

0 comments on commit a47e48a

Please sign in to comment.