Skip to content

Improve error message for one named, one anonymous lifetime parameters - underline Type #43851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gaurikholkar-zz wants to merge 2 commits into from
Closed

Improve error message for one named, one anonymous lifetime parameters - underline Type #43851

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4771970
Adding E0623 for structs
gaurikholkar Aug 5, 2017
a361a2c
changing error message format for E0611
gaurikholkar Aug 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 6 additions & 85 deletions src/librustc/infer/error_reporting/anon_anon_conflict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
//! where both the regions are anonymous.
use hir;
use infer::InferCtxt;
use ty::{self, Region};
use ty;
use infer::region_inference::RegionResolutionError::*;
use infer::region_inference::RegionResolutionError;
use hir::map as hir_map;
use middle::resolve_lifetime as rl;
use hir::intravisit::{self, Visitor, NestedVisitorMap};

Expand Down Expand Up @@ -86,9 +85,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
return false;
}




if anon_arg1 == anon_arg2 {
(format!(" with one lifetime"), format!(" into the other"))
} else {
Expand Down Expand Up @@ -119,81 +115,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
return true;

}

/// This function calls the `visit_ty` method for the parameters
/// corresponding to the anonymous regions. The `nested_visitor.found_type`
/// contains the anonymous type.
///
/// # Arguments
///
/// region - the anonymous region corresponding to the anon_anon conflict
/// br - the bound region corresponding to the above region which is of type `BrAnon(_)`
///
/// # Example
/// ```
/// fn foo(x: &mut Vec<&u8>, y: &u8)
/// { x.push(y); }
/// ```
/// The function returns the nested type corresponding to the anonymous region
/// for e.g. `&u8` and Vec<`&u8`.
pub fn find_anon_type(&self, region: Region<'tcx>, br: &ty::BoundRegion) -> Option<(&hir::Ty)> {
if let Some(anon_reg) = self.is_suitable_anonymous_region(region, true) {
let (def_id, _) = anon_reg;
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
let ret_ty = self.tcx.type_of(def_id);
if let ty::TyFnDef(_, _) = ret_ty.sty {
if let hir_map::NodeItem(it) = self.tcx.hir.get(node_id) {
if let hir::ItemFn(ref fndecl, _, _, _, _, _) = it.node {
return fndecl
.inputs
.iter()
.filter_map(|arg| {
self.find_visitor_found_type(&**arg, br)
})
.next();
}
} else if let hir_map::NodeTraitItem(it) = self.tcx.hir.get(node_id) {
if let hir::TraitItemKind::Method(ref fndecl, _) = it.node {
return fndecl
.decl
.inputs
.iter()
.filter_map(|arg| {
self.find_visitor_found_type(&**arg, br)
})
.next();
}
} else if let hir_map::NodeImplItem(it) = self.tcx.hir.get(node_id) {
if let hir::ImplItemKind::Method(ref fndecl, _) = it.node {
return fndecl
.decl
.inputs
.iter()
.filter_map(|arg| {
self.find_visitor_found_type(&**arg, br)
})
.next();
}
}
}
}
}
None
}

fn find_visitor_found_type(&self,
arg: &'gcx hir::Ty,
br: &ty::BoundRegion)
-> Option<(&'gcx hir::Ty)> {
let mut nested_visitor = FindNestedTypeVisitor {
infcx: &self,
hir_map: &self.tcx.hir,
bound_region: *br,
found_type: None,
};
nested_visitor.visit_ty(arg);
nested_visitor.found_type
}
}

// The FindNestedTypeVisitor captures the corresponding `hir::Ty` of the
Expand All @@ -203,15 +124,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// walk the types like &mut Vec<&u8> and &u8 looking for the HIR
// where that lifetime appears. This allows us to highlight the
// specific part of the type in the error message.
struct FindNestedTypeVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
hir_map: &'a hir::map::Map<'gcx>,
pub struct FindNestedTypeVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
pub infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
pub hir_map: &'a hir::map::Map<'gcx>,
// The bound_region corresponding to the Refree(freeregion)
// associated with the anonymous region we are looking for.
bound_region: ty::BoundRegion,
pub bound_region: ty::BoundRegion,
// The type where the anonymous lifetime appears
// for e.g. Vec<`&u8`> and <`&u8`>
found_type: Option<&'gcx hir::Ty>,
pub found_type: Option<&'gcx hir::Ty>,
}

impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -963,4 +963,5 @@ impl<'tcx> ObligationCause<'tcx> {
_ => "types are compatible",
}
}

}
17 changes: 12 additions & 5 deletions src/librustc/infer/error_reporting/named_anon_conflict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,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
let (named, (arg, new_ty, br, is_first), (scope_def_id, _), anon) = if
sub.is_named_region() && self.is_suitable_anonymous_region(sup, false).is_some() {
(sub,
self.find_arg_with_anonymous_region(sup, sub).unwrap(),
self.is_suitable_anonymous_region(sup, false).unwrap())
self.is_suitable_anonymous_region(sup, false).unwrap(),
sup)
} else if
sup.is_named_region() && self.is_suitable_anonymous_region(sub, false).is_some() {
(sup,
self.find_arg_with_anonymous_region(sub, sup).unwrap(),
self.is_suitable_anonymous_region(sub, false).unwrap())
self.is_suitable_anonymous_region(sub, false).unwrap(),
sub)
} else {
return false; // inapplicable
};
Expand All @@ -54,17 +56,22 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
("parameter type".to_owned(), "type".to_owned())
};

let span_label = if let Some(ty_anon) = self.find_anon_type(anon, &br) {
ty_anon.span
} else {
arg.pat.span
};

struct_span_err!(self.tcx.sess,
span,
E0621,
"explicit lifetime required in {}",
error_var)
.span_label(arg.pat.span,
.span_label(span_label,
format!("consider changing {} to `{}`", span_label_var, new_ty))
.span_label(span, format!("lifetime `{}` required", named))
.emit();


}
return true;
}
Expand Down
81 changes: 81 additions & 0 deletions src/librustc/infer/error_reporting/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use infer::InferCtxt;
use ty::{self, Region};
use hir::def_id::DefId;
use hir::map as hir_map;
use hir::intravisit::Visitor;
use infer::error_reporting::anon_anon_conflict::FindNestedTypeVisitor;

impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// This method walks the Type of the function body arguments using
Expand Down Expand Up @@ -140,6 +142,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
false
}

// 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
Expand All @@ -154,4 +157,82 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
false
}

/// This function calls the `visit_ty` method for the parameters
/// corresponding to the anonymous regions. The `nested_visitor.found_type`
/// contains the anonymous type.
///
/// # Arguments
///
/// region - the anonymous region corresponding to the lifetime conflicts
/// due to anonymous regions.
/// br - the bound region corresponding to the above region which is of type `BrAnon(_)`
///
/// # Example
/// ```
/// fn foo(x: &mut Vec<&u8>, y: &u8)
/// { x.push(y); }
/// ```
/// The function returns the nested type corresponding to the anonymous region
/// for e.g. `&u8` and Vec<`&u8`.
pub fn find_anon_type(&self, region: Region<'tcx>, br: &ty::BoundRegion) -> Option<(&hir::Ty)> {
if let Some(anon_reg) = self.is_suitable_anonymous_region(region, true) {
let (def_id, _) = anon_reg;
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
let ret_ty = self.tcx.type_of(def_id);
if let ty::TyFnDef(_, _) = ret_ty.sty {
if let hir_map::NodeItem(it) = self.tcx.hir.get(node_id) {
if let hir::ItemFn(ref fndecl, _, _, _, _, _) = it.node {
return fndecl
.inputs
.iter()
.filter_map(|arg| {
self.find_visitor_found_type(&**arg, br)
})
.next();
}
} else if let hir_map::NodeTraitItem(it) = self.tcx.hir.get(node_id) {
if let hir::TraitItemKind::Method(ref fndecl, _) = it.node {
return fndecl
.decl
.inputs
.iter()
.filter_map(|arg| {
self.find_visitor_found_type(&**arg, br)
})
.next();
}
} else if let hir_map::NodeImplItem(it) = self.tcx.hir.get(node_id) {
if let hir::ImplItemKind::Method(ref fndecl, _) = it.node {
return fndecl
.decl
.inputs
.iter()
.filter_map(|arg| {
self.find_visitor_found_type(&**arg, br)
})
.next();
}
}
}
}
}
None
}

// This method creates a FindNestedTypeVisitor which returns the
// corresponding anonymous region.
fn find_visitor_found_type(&self,
arg: &'gcx hir::Ty,
br: &ty::BoundRegion)
-> Option<(&'gcx hir::Ty)> {
let mut nested_visitor = FindNestedTypeVisitor {
infcx: &self,
hir_map: &self.tcx.hir,
bound_region: *br,
found_type: None,
};
nested_visitor.visit_ty(arg);
nested_visitor.found_type
}
}
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 {
| - 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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-3.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 {
| ------ consider changing type to `(&'a i32, &'a i32)`
| ---- consider changing type to `(&'a i32, &'a i32)`
Copy link
Contributor

@nikomatsakis nikomatsakis Aug 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so here we are underlining only a piece of the type, but the replacement that we are suggesting is the entire type. This seems a bit confusing to me.

12 | if x > y { x } else { y }
| ^ lifetime `'a` required

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 {
| - consider changing the type of `x` to `&'a i32`
| ---- consider changing the type of `x` to `&'a i32`
14 | if x > y { x } else { y }
| ^ lifetime `'a` required

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else-using-impl-3.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 {
| - consider changing the type of `x` to `&'a i32`
| ---- consider changing the type of `x` to `&'a i32`
17 |
18 | if true { &self.field } else { x }
| ^ lifetime `'a` required
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex1-return-one-existing-name-if-else.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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 {
| - 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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex2a-push-one-existing-name-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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<i32>, y: &mut Vec<Ref<'a, i32>>) {
| - 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

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lifetime-errors/ex2a-push-one-existing-name.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ 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<Ref<'a, i32>>, y: Ref<i32>) {
| - 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

Expand Down