Skip to content

Commit 14c6408

Browse files
nikomatsakisnikomatsakis
committed
detect when OldStyleLUB would cause a different error
1 parent cf2f2a4 commit 14c6408

File tree

5 files changed

+302
-11
lines changed

5 files changed

+302
-11
lines changed

‎src/librustc/infer/glb.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use super::Subtype;
1515

1616
use traits::ObligationCause;
1717
use ty::{self, Ty, TyCtxt};
18+
use ty::error::TypeError;
1819
use ty::relate::{Relate, RelateResult, TypeRelation};
1920

2021
/// "Greatest lower bound" (common subtype)
@@ -74,10 +75,23 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
7475
-> RelateResult<'tcx, ty::Binder<T>>
7576
where T: Relate<'tcx>
7677
{
77-
// Otherwise, we make the (overly strict) requirement that
78-
// the two sides are equal.
79-
self.relate_with_variance(ty::Variance::Invariant, a, b)?;
80-
Ok(a.clone())
78+
let was_error = self.infcx().probe(|_snapshot| {
79+
self.fields.higher_ranked_glb(a, b, self.a_is_expected).is_ok()
80+
});
81+
82+
// When higher-ranked types are involved, computing the LUB is
83+
// very challenging, switch to invariance. This is obviously
84+
// overly conservative but works ok in practice.
85+
match self.relate_with_variance(ty::Variance::Invariant, a, b) {
86+
Ok(_) => Ok(a.clone()),
87+
Err(err) => {
88+
if !was_error {
89+
Err(TypeError::OldStyleLUB(Box::new(err)))
90+
} else {
91+
Err(err)
92+
}
93+
}
94+
}
8195
}
8296
}
8397

‎src/librustc/infer/higher_ranked/mod.rs

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,261 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
198198
Ok(HrMatchResult { value: a_value })
199199
});
200200
}
201+
202+
pub fn higher_ranked_lub<T>(&mut self, a: &Binder<T>, b: &Binder<T>, a_is_expected: bool)
203+
-> RelateResult<'tcx, Binder<T>>
204+
where T: Relate<'tcx>
205+
{
206+
// Start a snapshot so we can examine "all bindings that were
207+
// created as part of this type comparison".
208+
return self.infcx.commit_if_ok(|snapshot| {
209+
// Instantiate each bound region with a fresh region variable.
210+
let span = self.trace.cause.span;
211+
let (a_with_fresh, a_map) =
212+
self.infcx.replace_late_bound_regions_with_fresh_var(
213+
span, HigherRankedType, a);
214+
let (b_with_fresh, _) =
215+
self.infcx.replace_late_bound_regions_with_fresh_var(
216+
span, HigherRankedType, b);
217+
218+
// Collect constraints.
219+
let result0 =
220+
self.lub(a_is_expected).relate(&a_with_fresh, &b_with_fresh)?;
221+
let result0 =
222+
self.infcx.resolve_type_vars_if_possible(&result0);
223+
debug!("lub result0 = {:?}", result0);
224+
225+
// Generalize the regions appearing in result0 if possible
226+
let new_vars = self.infcx.region_vars_confined_to_snapshot(snapshot);
227+
let span = self.trace.cause.span;
228+
let result1 =
229+
fold_regions_in(
230+
self.tcx(),
231+
&result0,
232+
|r, debruijn| generalize_region(self.infcx, span, snapshot, debruijn,
233+
&new_vars, &a_map, r));
234+
235+
debug!("lub({:?},{:?}) = {:?}",
236+
a,
237+
b,
238+
result1);
239+
240+
Ok(ty::Binder(result1))
241+
});
242+
243+
fn generalize_region<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
244+
span: Span,
245+
snapshot: &CombinedSnapshot,
246+
debruijn: ty::DebruijnIndex,
247+
new_vars: &[ty::RegionVid],
248+
a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
249+
r0: ty::Region<'tcx>)
250+
-> ty::Region<'tcx> {
251+
// Regions that pre-dated the LUB computation stay as they are.
252+
if !is_var_in_set(new_vars, r0) {
253+
assert!(!r0.is_late_bound());
254+
debug!("generalize_region(r0={:?}): not new variable", r0);
255+
return r0;
256+
}
257+
258+
let tainted = infcx.tainted_regions(snapshot, r0, TaintDirections::both());
259+
260+
// Variables created during LUB computation which are
261+
// *related* to regions that pre-date the LUB computation
262+
// stay as they are.
263+
if !tainted.iter().all(|&r| is_var_in_set(new_vars, r)) {
264+
debug!("generalize_region(r0={:?}): \
265+
non-new-variables found in {:?}",
266+
r0, tainted);
267+
assert!(!r0.is_late_bound());
268+
return r0;
269+
}
270+
271+
// Otherwise, the variable must be associated with at
272+
// least one of the variables representing bound regions
273+
// in both A and B. Replace the variable with the "first"
274+
// bound region from A that we find it to be associated
275+
// with.
276+
for (a_br, a_r) in a_map {
277+
if tainted.iter().any(|x| x == a_r) {
278+
debug!("generalize_region(r0={:?}): \
279+
replacing with {:?}, tainted={:?}",
280+
r0, *a_br, tainted);
281+
return infcx.tcx.mk_region(ty::ReLateBound(debruijn, *a_br));
282+
}
283+
}
284+
285+
span_bug!(
286+
span,
287+
"region {:?} is not associated with any bound region from A!",
288+
r0)
289+
}
290+
}
291+
292+
pub fn higher_ranked_glb<T>(&mut self, a: &Binder<T>, b: &Binder<T>, a_is_expected: bool)
293+
-> RelateResult<'tcx, Binder<T>>
294+
where T: Relate<'tcx>
295+
{
296+
debug!("higher_ranked_glb({:?}, {:?})",
297+
a, b);
298+
299+
// Make a snapshot so we can examine "all bindings that were
300+
// created as part of this type comparison".
301+
return self.infcx.commit_if_ok(|snapshot| {
302+
// Instantiate each bound region with a fresh region variable.
303+
let (a_with_fresh, a_map) =
304+
self.infcx.replace_late_bound_regions_with_fresh_var(
305+
self.trace.cause.span, HigherRankedType, a);
306+
let (b_with_fresh, b_map) =
307+
self.infcx.replace_late_bound_regions_with_fresh_var(
308+
self.trace.cause.span, HigherRankedType, b);
309+
let a_vars = var_ids(self, &a_map);
310+
let b_vars = var_ids(self, &b_map);
311+
312+
// Collect constraints.
313+
let result0 =
314+
self.glb(a_is_expected).relate(&a_with_fresh, &b_with_fresh)?;
315+
let result0 =
316+
self.infcx.resolve_type_vars_if_possible(&result0);
317+
debug!("glb result0 = {:?}", result0);
318+
319+
// Generalize the regions appearing in result0 if possible
320+
let new_vars = self.infcx.region_vars_confined_to_snapshot(snapshot);
321+
let span = self.trace.cause.span;
322+
let result1 =
323+
fold_regions_in(
324+
self.tcx(),
325+
&result0,
326+
|r, debruijn| generalize_region(self.infcx, span, snapshot, debruijn,
327+
&new_vars,
328+
&a_map, &a_vars, &b_vars,
329+
r));
330+
331+
debug!("glb({:?},{:?}) = {:?}",
332+
a,
333+
b,
334+
result1);
335+
336+
Ok(ty::Binder(result1))
337+
});
338+
339+
fn generalize_region<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
340+
span: Span,
341+
snapshot: &CombinedSnapshot,
342+
debruijn: ty::DebruijnIndex,
343+
new_vars: &[ty::RegionVid],
344+
a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
345+
a_vars: &[ty::RegionVid],
346+
b_vars: &[ty::RegionVid],
347+
r0: ty::Region<'tcx>)
348+
-> ty::Region<'tcx> {
349+
if !is_var_in_set(new_vars, r0) {
350+
assert!(!r0.is_late_bound());
351+
return r0;
352+
}
353+
354+
let tainted = infcx.tainted_regions(snapshot, r0, TaintDirections::both());
355+
356+
let mut a_r = None;
357+
let mut b_r = None;
358+
let mut only_new_vars = true;
359+
for r in &tainted {
360+
if is_var_in_set(a_vars, *r) {
361+
if a_r.is_some() {
362+
return fresh_bound_variable(infcx, debruijn);
363+
} else {
364+
a_r = Some(*r);
365+
}
366+
} else if is_var_in_set(b_vars, *r) {
367+
if b_r.is_some() {
368+
return fresh_bound_variable(infcx, debruijn);
369+
} else {
370+
b_r = Some(*r);
371+
}
372+
} else if !is_var_in_set(new_vars, *r) {
373+
only_new_vars = false;
374+
}
375+
}
376+
377+
// NB---I do not believe this algorithm computes
378+
// (necessarily) the GLB. As written it can
379+
// spuriously fail. In particular, if there is a case
380+
// like: |fn(&a)| and fn(fn(&b)), where a and b are
381+
// free, it will return fn(&c) where c = GLB(a,b). If
382+
// however this GLB is not defined, then the result is
383+
// an error, even though something like
384+
// "fn<X>(fn(&X))" where X is bound would be a
385+
// subtype of both of those.
386+
//
387+
// The problem is that if we were to return a bound
388+
// variable, we'd be computing a lower-bound, but not
389+
// necessarily the *greatest* lower-bound.
390+
//
391+
// Unfortunately, this problem is non-trivial to solve,
392+
// because we do not know at the time of computing the GLB
393+
// whether a GLB(a,b) exists or not, because we haven't
394+
// run region inference (or indeed, even fully computed
395+
// the region hierarchy!). The current algorithm seems to
396+
// works ok in practice.
397+
398+
if a_r.is_some() && b_r.is_some() && only_new_vars {
399+
// Related to exactly one bound variable from each fn:
400+
return rev_lookup(infcx, span, a_map, a_r.unwrap());
401+
} else if a_r.is_none() && b_r.is_none() {
402+
// Not related to bound variables from either fn:
403+
assert!(!r0.is_late_bound());
404+
return r0;
405+
} else {
406+
// Other:
407+
return fresh_bound_variable(infcx, debruijn);
408+
}
409+
}
410+
411+
fn rev_lookup<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
412+
span: Span,
413+
a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
414+
r: ty::Region<'tcx>) -> ty::Region<'tcx>
415+
{
416+
for (a_br, a_r) in a_map {
417+
if *a_r == r {
418+
return infcx.tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), *a_br));
419+
}
420+
}
421+
span_bug!(
422+
span,
423+
"could not find original bound region for {:?}",
424+
r);
425+
}
426+
427+
fn fresh_bound_variable<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
428+
debruijn: ty::DebruijnIndex)
429+
-> ty::Region<'tcx> {
430+
infcx.region_vars.new_bound(debruijn)
431+
}
432+
}
433+
}
434+
435+
fn var_ids<'a, 'gcx, 'tcx>(fields: &CombineFields<'a, 'gcx, 'tcx>,
436+
map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
437+
-> Vec<ty::RegionVid> {
438+
map.iter()
439+
.map(|(_, &r)| match *r {
440+
ty::ReVar(r) => { r }
441+
_ => {
442+
span_bug!(
443+
fields.trace.cause.span,
444+
"found non-region-vid: {:?}",
445+
r);
446+
}
447+
})
448+
.collect()
449+
}
450+
451+
fn is_var_in_set(new_vars: &[ty::RegionVid], r: ty::Region) -> bool {
452+
match *r {
453+
ty::ReVar(ref v) => new_vars.iter().any(|x| x == v),
454+
_ => false
455+
}
201456
}
202457

203458
fn fold_regions_in<'a, 'gcx, 'tcx, T, F>(tcx: TyCtxt<'a, 'gcx, 'tcx>,

‎src/librustc/infer/lub.rs

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
use super::InferCtxt;
12-
use super::Subtype;
1311
use super::combine::CombineFields;
12+
use super::InferCtxt;
1413
use super::lattice::{self, LatticeDir};
14+
use super::Subtype;
1515

1616
use traits::ObligationCause;
1717
use ty::{self, Ty, TyCtxt};
18+
use ty::error::TypeError;
1819
use ty::relate::{Relate, RelateResult, TypeRelation};
1920

2021
/// "Least upper bound" (common supertype)
@@ -74,10 +75,23 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
7475
-> RelateResult<'tcx, ty::Binder<T>>
7576
where T: Relate<'tcx>
7677
{
77-
// Otherwise, we make the (overly strict) requirement that
78-
// the two sides are equal.
79-
self.relate_with_variance(ty::Variance::Invariant, a, b)?;
80-
Ok(a.clone())
78+
let was_error = self.infcx().probe(|_snapshot| {
79+
self.fields.higher_ranked_lub(a, b, self.a_is_expected).is_ok()
80+
});
81+
82+
// When higher-ranked types are involved, computing the LUB is
83+
// very challenging, switch to invariance. This is obviously
84+
// overly conservative but works ok in practice.
85+
match self.relate_with_variance(ty::Variance::Invariant, a, b) {
86+
Ok(_) => Ok(a.clone()),
87+
Err(err) => {
88+
if !was_error {
89+
Err(TypeError::OldStyleLUB(Box::new(err)))
90+
} else {
91+
Err(err)
92+
}
93+
}
94+
}
8195
}
8296
}
8397

‎src/librustc/ty/error.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ pub enum TypeError<'tcx> {
5454
ProjectionBoundsLength(ExpectedFound<usize>),
5555
TyParamDefaultMismatch(ExpectedFound<type_variable::Default<'tcx>>),
5656
ExistentialMismatch(ExpectedFound<&'tcx ty::Slice<ty::ExistentialPredicate<'tcx>>>),
57+
58+
OldStyleLUB(Box<TypeError<'tcx>>),
5759
}
5860

5961
#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Hash, Debug, Copy)]
@@ -170,6 +172,9 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
170172
report_maybe_different(f, format!("trait `{}`", values.expected),
171173
format!("trait `{}`", values.found))
172174
}
175+
OldStyleLUB(ref err) => {
176+
write!(f, "{}", err)
177+
}
173178
}
174179
}
175180
}

‎src/librustc/ty/structural_impls.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
428428
TyParamDefaultMismatch(ref x) => {
429429
return tcx.lift(x).map(TyParamDefaultMismatch)
430430
}
431-
ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch)
431+
ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch),
432+
OldStyleLUB(ref x) => return tcx.lift(x).map(OldStyleLUB),
432433
})
433434
}
434435
}
@@ -1174,6 +1175,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::error::TypeError<'tcx> {
11741175
Sorts(x) => Sorts(x.fold_with(folder)),
11751176
TyParamDefaultMismatch(ref x) => TyParamDefaultMismatch(x.fold_with(folder)),
11761177
ExistentialMismatch(x) => ExistentialMismatch(x.fold_with(folder)),
1178+
OldStyleLUB(ref x) => OldStyleLUB(x.fold_with(folder)),
11771179
}
11781180
}
11791181

@@ -1191,6 +1193,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::error::TypeError<'tcx> {
11911193
b.visit_with(visitor)
11921194
},
11931195
Sorts(x) => x.visit_with(visitor),
1196+
OldStyleLUB(ref x) => x.visit_with(visitor),
11941197
TyParamDefaultMismatch(ref x) => x.visit_with(visitor),
11951198
ExistentialMismatch(x) => x.visit_with(visitor),
11961199
Mismatch |

0 commit comments

Comments
 (0)