Skip to content

Commit e58f528

Browse files
gaurikholkarnikomatsakis
gaurikholkar
authored and
nikomatsakis
committed
merge fixes, addressing CR comments
1 parent 1c4510a commit e58f528

7 files changed

+138
-158
lines changed

‎src/librustc/infer/error_reporting/different_lifetimes.rs

Lines changed: 80 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use infer::region_inference::RegionResolutionError;
1818
use hir::map as hir_map;
1919
use middle::resolve_lifetime as rl;
2020
use hir::intravisit::{self, Visitor, NestedVisitorMap};
21+
use infer::error_reporting::util::AnonymousArgInfo;
2122

2223
impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
2324
// This method prints the error message for lifetime errors when both the concerned regions
@@ -57,6 +58,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
5758
let ty_sup = or_false!(self.find_anon_type(sup, &bregion_sup));
5859

5960
let ty_sub = or_false!(self.find_anon_type(sub, &bregion_sub));
61+
6062
debug!("try_report_anon_anon_conflict: found_arg1={:?} sup={:?} br1={:?}",
6163
ty_sub,
6264
sup,
@@ -66,56 +68,78 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
6668
sub,
6769
bregion_sub);
6870

69-
let (main_label, label1, label2) = if let (Some(sup_arg), Some(sub_arg)) =
70-
(self.find_arg_with_region(sup, sup), self.find_arg_with_region(sub, sub)) {
71+
let (ty_sup, ty_fndecl_sup) = ty_sup;
72+
let (ty_sub, ty_fndecl_sub) = ty_sub;
7173

72-
let (anon_arg_sup, is_first_sup, anon_arg_sub, is_first_sub) =
73-
(sup_arg.arg, sup_arg.is_first, sub_arg.arg, sub_arg.is_first);
74-
if self.is_self_anon(is_first_sup, scope_def_id_sup) ||
75-
self.is_self_anon(is_first_sub, scope_def_id_sub) {
76-
return false;
77-
}
74+
let AnonymousArgInfo { arg: anon_arg_sup, .. } =
75+
or_false!(self.find_arg_with_region(sup, sup));
76+
let AnonymousArgInfo { arg: anon_arg_sub, .. } =
77+
or_false!(self.find_arg_with_region(sub, sub));
7878

79-
if self.is_return_type_anon(scope_def_id_sup, bregion_sup) ||
80-
self.is_return_type_anon(scope_def_id_sub, bregion_sub) {
81-
return false;
82-
}
79+
let sup_is_ret_type =
80+
self.is_return_type_anon(scope_def_id_sup, bregion_sup, ty_fndecl_sup);
81+
let sub_is_ret_type =
82+
self.is_return_type_anon(scope_def_id_sub, bregion_sub, ty_fndecl_sub);
8383

84-
if anon_arg_sup == anon_arg_sub {
85-
(format!("this type was declared with multiple lifetimes..."),
86-
format!(" with one lifetime"),
87-
format!(" into the other"))
88-
} else {
89-
let span_label_var1 = if let Some(simple_name) = anon_arg_sup.pat.simple_name() {
90-
format!(" from `{}`", simple_name)
91-
} else {
92-
format!("")
93-
};
84+
let span_label_var1 = if let Some(simple_name) = anon_arg_sup.pat.simple_name() {
85+
format!(" flows from `{}`", simple_name)
86+
} else {
87+
format!("")
88+
};
89+
90+
let span_label_var2 = if let Some(simple_name) = anon_arg_sub.pat.simple_name() {
91+
format!(" into `{}`", simple_name)
92+
} else {
93+
format!("")
94+
};
9495

95-
let span_label_var2 = if let Some(simple_name) = anon_arg_sub.pat.simple_name() {
96-
format!(" into `{}`", simple_name)
97-
} else {
98-
format!("")
99-
};
10096

101-
let span_label =
102-
format!("these two types are declared with different lifetimes...",);
97+
let (span_1, span_2, main_label, span_label) = match (sup_is_ret_type, sub_is_ret_type) {
98+
(None, None) => {
99+
let (main_label_1, span_label_1) = if ty_sup == ty_sub {
103100

104-
(span_label, span_label_var1, span_label_var2)
101+
(format!("this type was declared with multiple lifetimes..."),
102+
format!("...but data{} flows{} here",
103+
format!(" with one lifetime"),
104+
format!(" into the other")))
105+
} else {
106+
(format!("these two types was declared with multiple lifetimes..."),
107+
format!("...but data{} flows{} here",
108+
span_label_var1,
109+
span_label_var2))
110+
};
111+
(ty_sup.span, ty_sub.span, main_label_1, span_label_1)
112+
}
113+
(Some(ret_span1), Some(ret_span2)) => {
114+
(ret_span1,
115+
ret_span2,
116+
format!("the return type is declared with different lifetimes..."),
117+
format!("...but data{} flows{} here",
118+
format!(" with one lifetime"),
119+
format!(" into the other")))
120+
}
121+
122+
(Some(ret_span), _) => {
123+
(ty_sub.span,
124+
ret_span,
125+
format!("this parameter and the return type are declared
126+
with different lifetimes...",),
127+
format!("...but data{} is returned here", span_label_var1))
128+
}
129+
(_, Some(ret_span)) => {
130+
(ty_sup.span,
131+
ret_span,
132+
format!("this parameter and the return type are declared
133+
with different lifetimes...",),
134+
format!("...but data{} is returned here", span_label_var1))
105135
}
106-
} else {
107-
debug!("no arg with anon region found");
108-
debug!("try_report_anon_anon_conflict: is_suitable(sub) = {:?}",
109-
self.is_suitable_region(sub));
110-
debug!("try_report_anon_anon_conflict: is_suitable(sup) = {:?}",
111-
self.is_suitable_region(sup));
112-
return false;
113136
};
114137

138+
115139
struct_span_err!(self.tcx.sess, span, E0623, "lifetime mismatch")
116-
.span_label(ty_sup.span, main_label)
117-
.span_label(ty_sub.span, format!(""))
118-
.span_label(span, format!("...but data{} flows{} here", label1, label2))
140+
.span_label(span_1, main_label)
141+
.span_label(span_2, format!(""))
142+
.span_label(span, span_label)
119143
.emit();
120144
return true;
121145
}
@@ -135,28 +159,32 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
135159
/// ```
136160
/// The function returns the nested type corresponding to the anonymous region
137161
/// for e.g. `&u8` and Vec<`&u8`.
138-
pub fn find_anon_type(&self, region: Region<'tcx>, br: &ty::BoundRegion) -> Option<&hir::Ty> {
162+
pub fn find_anon_type(&self,
163+
region: Region<'tcx>,
164+
br: &ty::BoundRegion)
165+
-> Option<(&hir::Ty, &hir::FnDecl)> {
139166
if let Some(anon_reg) = self.is_suitable_region(region) {
140167
let def_id = anon_reg.def_id;
141168
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
142-
let inputs: &[_] = match self.tcx.hir.get(node_id) {
169+
let fndecl = match self.tcx.hir.get(node_id) {
143170
hir_map::NodeItem(&hir::Item { node: hir::ItemFn(ref fndecl, ..), .. }) => {
144-
&fndecl.inputs
171+
&fndecl
145172
}
146173
hir_map::NodeTraitItem(&hir::TraitItem {
147-
node: hir::TraitItemKind::Method(ref fndecl, ..), ..
148-
}) => &fndecl.decl.inputs,
174+
node: hir::TraitItemKind::Method(ref m, ..), ..
175+
}) |
149176
hir_map::NodeImplItem(&hir::ImplItem {
150-
node: hir::ImplItemKind::Method(ref fndecl, ..), ..
151-
}) => &fndecl.decl.inputs,
152-
153-
_ => &[],
177+
node: hir::ImplItemKind::Method(ref m, ..), ..
178+
}) => &m.decl,
179+
_ => return None,
154180
};
155181

156-
return inputs
182+
return fndecl
183+
.inputs
157184
.iter()
158-
.filter_map(|arg| self.find_component_for_bound_region(&**arg, br))
159-
.next();
185+
.filter_map(|arg| self.find_component_for_bound_region(arg, br))
186+
.next()
187+
.map(|ty| (ty, &**fndecl));
160188
}
161189
}
162190
None

‎src/librustc/infer/error_reporting/named_anon_conflict.rs

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
3535
// only introduced anonymous regions in parameters) as well as a
3636
// version new_ty of its type where the anonymous region is replaced
3737
// with the named one.//scope_def_id
38-
let (named, anon_arg_info, region_info) =
38+
let (named, anon, anon_arg_info, region_info) =
3939
if self.is_named_region(sub) && self.is_suitable_region(sup).is_some() &&
4040
self.find_arg_with_region(sup, sub).is_some() {
4141
(sub,
42+
sup,
4243
self.find_arg_with_region(sup, sub).unwrap(),
4344
self.is_suitable_region(sup).unwrap())
4445
} else if self.is_named_region(sup) && self.is_suitable_region(sub).is_some() &&
4546
self.find_arg_with_region(sub, sup).is_some() {
4647
(sup,
48+
sub,
4749
self.find_arg_with_region(sub, sup).unwrap(),
4850
self.is_suitable_region(sub).unwrap())
4951
} else {
@@ -76,33 +78,30 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
7678
return false;
7779
}
7880

79-
if self.is_return_type_anon(scope_def_id, br) {
80-
debug!("try_report_named_anon_conflict: is_return_type_anon({:?}, {:?}) = true",
81-
scope_def_id,
82-
br);
83-
return false;
84-
} else if self.is_self_anon(is_first, scope_def_id) {
85-
debug!("try_report_named_anon_conflict: is_self_anon({:?}, {:?}) = true",
86-
is_first,
87-
scope_def_id);
88-
return false;
81+
if let Some(anon_ty) = self.find_anon_type(anon, &br) {
82+
let (_, fndecl) = anon_ty;
83+
if self.is_return_type_anon(scope_def_id, br, fndecl).is_some() ||
84+
self.is_self_anon(is_first, scope_def_id) {
85+
return false;
86+
}
87+
}
88+
89+
let (error_var, span_label_var) = if let Some(simple_name) = arg.pat.simple_name() {
90+
(format!("the type of `{}`", simple_name), format!("the type of `{}`", simple_name))
8991
} else {
90-
let (error_var, span_label_var) = if let Some(simple_name) = arg.pat.simple_name() {
91-
(format!("the type of `{}`", simple_name), format!("the type of `{}`", simple_name))
92-
} else {
93-
("parameter type".to_owned(), "type".to_owned())
94-
};
92+
("parameter type".to_owned(), "type".to_owned())
93+
};
94+
95+
struct_span_err!(self.tcx.sess,
96+
span,
97+
E0621,
98+
"explicit lifetime required in {}",
99+
error_var)
100+
.span_label(arg.pat.span,
101+
format!("consider changing {} to `{}`", span_label_var, new_ty))
102+
.span_label(span, format!("lifetime `{}` required", named))
103+
.emit();
104+
return true;
95105

96-
struct_span_err!(self.tcx.sess,
97-
span,
98-
E0621,
99-
"explicit lifetime required in {}",
100-
error_var)
101-
.span_label(arg.pat.span,
102-
format!("consider changing {} to `{}`", span_label_var, new_ty))
103-
.span_label(span, format!("lifetime `{}` required", named))
104-
.emit();
105-
return true;
106-
}
107106
}
108107
}

‎src/librustc/infer/error_reporting/util.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use infer::InferCtxt;
1515
use ty::{self, Region, Ty};
1616
use hir::def_id::DefId;
1717
use hir::map as hir_map;
18+
use syntax_pos::Span;
1819

1920
macro_rules! or_false {
2021
($v:expr) => {
@@ -163,20 +164,24 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
163164
// Here, we check for the case where the anonymous region
164165
// is in the return type.
165166
// FIXME(#42703) - Need to handle certain cases here.
166-
pub fn is_return_type_anon(&self, scope_def_id: DefId, br: ty::BoundRegion) -> bool {
167+
pub fn is_return_type_anon(&self,
168+
scope_def_id: DefId,
169+
br: ty::BoundRegion,
170+
decl: &hir::FnDecl)
171+
-> Option<Span> {
167172
let ret_ty = self.tcx.type_of(scope_def_id);
168173
match ret_ty.sty {
169174
ty::TyFnDef(_, _) => {
170175
let sig = ret_ty.fn_sig(self.tcx);
171176
let late_bound_regions = self.tcx
172177
.collect_referenced_late_bound_regions(&sig.output());
173178
if late_bound_regions.iter().any(|r| *r == br) {
174-
return true;
179+
return Some(decl.output.span());
175180
}
176181
}
177182
_ => {}
178183
}
179-
false
184+
None
180185
}
181186
// Here we check for the case where anonymous region
182187
// corresponds to self and if yes, we display E0312.

‎src/test/ui/lifetime-errors/ex1-return-one-existing-name-return-type-is-anon.stderr

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,11 @@
1-
error[E0312]: lifetime of reference outlives lifetime of borrowed content...
1+
error[E0621]: explicit lifetime required in the type of `self`
22
--> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:18:5
33
|
4+
16 | fn foo<'a>(&self, x: &'a i32) -> &i32 {
5+
| ----- consider changing the type of `self` to `&'a Foo`
6+
17 |
47
18 | x
5-
| ^
6-
|
7-
note: ...the reference is valid for the anonymous lifetime #1 defined on the method body at 16:3...
8-
--> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:16:3
9-
|
10-
16 | / fn foo<'a>(&self, x: &'a i32) -> &i32 {
11-
17 | |
12-
18 | | x
13-
19 | |
14-
20 | | }
15-
| |___^
16-
note: ...but the borrowed content is only valid for the lifetime 'a as defined on the method body at 16:3
17-
--> $DIR/ex1-return-one-existing-name-return-type-is-anon.rs:16:3
18-
|
19-
16 | / fn foo<'a>(&self, x: &'a i32) -> &i32 {
20-
17 | |
21-
18 | | x
22-
19 | |
23-
20 | | }
24-
| |___^
8+
| ^ lifetime `'a` required
259

2610
error: aborting due to previous error
2711

‎src/test/ui/lifetime-errors/ex1-return-one-existing-name-self-is-anon.stderr

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,11 @@
1-
error[E0312]: lifetime of reference outlives lifetime of borrowed content...
1+
error[E0621]: explicit lifetime required in the type of `self`
22
--> $DIR/ex1-return-one-existing-name-self-is-anon.rs:18:30
33
|
4+
16 | fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
5+
| ----- consider changing the type of `self` to `&'a Foo`
6+
17 |
47
18 | if true { x } else { self }
5-
| ^^^^
6-
|
7-
note: ...the reference is valid for the lifetime 'a as defined on the method body at 16:5...
8-
--> $DIR/ex1-return-one-existing-name-self-is-anon.rs:16:5
9-
|
10-
16 | / fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
11-
17 | |
12-
18 | | if true { x } else { self }
13-
19 | |
14-
20 | | }
15-
| |_____^
16-
note: ...but the borrowed content is only valid for the anonymous lifetime #1 defined on the method body at 16:5
17-
--> $DIR/ex1-return-one-existing-name-self-is-anon.rs:16:5
18-
|
19-
16 | / fn foo<'a>(&self, x: &'a Foo) -> &'a Foo {
20-
17 | |
21-
18 | | if true { x } else { self }
22-
19 | |
23-
20 | | }
24-
| |_____^
8+
| ^^^^ lifetime `'a` required
259

2610
error: aborting due to previous error
2711

‎src/test/ui/lifetime-errors/ex3-both-anon-regions-return-type-is-anon.stderr

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,13 @@
1-
error[E0312]: lifetime of reference outlives lifetime of borrowed content...
1+
error[E0621]: explicit lifetime required in the type of `self`
22
--> $DIR/ex3-both-anon-regions-return-type-is-anon.rs:17:5
33
|
4+
16 | fn foo<'a>(&self, x: &i32) -> &i32 {
5+
| ---- ----
6+
| |
7+
| this parameter and the return type are
8+
declared with different lifetimes...
49
17 | x
5-
| ^
6-
|
7-
note: ...the reference is valid for the anonymous lifetime #1 defined on the method body at 16:3...
8-
--> $DIR/ex3-both-anon-regions-return-type-is-anon.rs:16:3
9-
|
10-
16 | / fn foo<'a>(&self, x: &i32) -> &i32 {
11-
17 | | x
12-
18 | | }
13-
| |___^
14-
note: ...but the borrowed content is only valid for the anonymous lifetime #2 defined on the method body at 16:3
15-
--> $DIR/ex3-both-anon-regions-return-type-is-anon.rs:16:3
16-
|
17-
16 | / fn foo<'a>(&self, x: &i32) -> &i32 {
18-
17 | | x
19-
18 | | }
20-
| |___^
10+
| ^ ...but data from `x` is returned here
2111

2212
error: aborting due to previous error
2313

0 commit comments

Comments
 (0)