Skip to content

Commit c86d99d

Browse files
borsbors
committed
Auto merge of #45853 - nikomatsakis:chalk-simplify-hr-lub-glb, r=arielb1
Simplify higher-ranked LUB/GLB This is a better version of #44211. It still makes higher-ranked LUB/GLB into a hard equality test, however, it does try to identify that something changed and issue a notice to the user. I wroteup #45852 as a tracking issue for this change. Currently, this moves straight to a hard-error, on the basis that the crater run in #44211 saw no impact. It might be good to retest -- or perhaps to try for a warning period. Trying to do the latter in a precise way would be somewhat painful, but an imprecise way might suffice -- that is, we could issue warning *whenever* a LUB/GLB operation succeeds that will later fail, even if it doesn't ultimately impact the type check. I could experiment with this. ~~I am *mildly* wary about landing this independently of other code that moves to a universe-based system. In particular, I was nervous that this change would make coherence accepts new pairs of impls that will later be errors. I have the code for the universe-based approach available, I hope to open an PR and run some tests on its impact very shortly.~~ @arielb1 points out that I was being silly. r? @arielb1
2 parents aabfed5 + 9877fa0 commit c86d99d

File tree

14 files changed

+232
-226
lines changed

14 files changed

+232
-226
lines changed

Diff for: ‎src/librustc/infer/error_reporting/mod.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -762,16 +762,23 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
762762
}
763763
}
764764

765-
self.note_error_origin(diag, &cause);
766765
self.check_and_note_conflicting_crates(diag, terr, span);
767766
self.tcx.note_and_explain_type_err(diag, terr, span);
767+
768+
// It reads better to have the error origin as the final
769+
// thing.
770+
self.note_error_origin(diag, &cause);
768771
}
769772

770773
pub fn report_and_explain_type_error(&self,
771774
trace: TypeTrace<'tcx>,
772775
terr: &TypeError<'tcx>)
773776
-> DiagnosticBuilder<'tcx>
774777
{
778+
debug!("report_and_explain_type_error(trace={:?}, terr={:?})",
779+
trace,
780+
terr);
781+
775782
let span = trace.cause.span;
776783
let failure_str = trace.cause.as_failure_str();
777784
let mut diag = match trace.cause.code {

Diff for: ‎src/librustc/infer/glb.rs

+27-1
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,7 +75,32 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
7475
-> RelateResult<'tcx, ty::Binder<T>>
7576
where T: Relate<'tcx>
7677
{
77-
self.fields.higher_ranked_glb(a, b, self.a_is_expected)
78+
debug!("binders(a={:?}, b={:?})", a, b);
79+
let was_error = self.infcx().probe(|_snapshot| {
80+
// Subtle: use a fresh combine-fields here because we recover
81+
// from Err. Doing otherwise could propagate obligations out
82+
// through our `self.obligations` field.
83+
self.infcx()
84+
.combine_fields(self.fields.trace.clone(), self.fields.param_env)
85+
.higher_ranked_glb(a, b, self.a_is_expected)
86+
.is_err()
87+
});
88+
debug!("binders: was_error={:?}", was_error);
89+
90+
// When higher-ranked types are involved, computing the LUB is
91+
// very challenging, switch to invariance. This is obviously
92+
// overly conservative but works ok in practice.
93+
match self.relate_with_variance(ty::Variance::Invariant, a, b) {
94+
Ok(_) => Ok(a.clone()),
95+
Err(err) => {
96+
debug!("binders: error occurred, was_error={:?}", was_error);
97+
if !was_error {
98+
Err(TypeError::OldStyleLUB(Box::new(err)))
99+
} else {
100+
Err(err)
101+
}
102+
}
103+
}
78104
}
79105
}
80106

Diff for: ‎src/librustc/infer/higher_ranked/mod.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use super::{CombinedSnapshot,
1919
use super::combine::CombineFields;
2020
use super::region_constraints::{TaintDirections};
2121

22+
use std::collections::BTreeMap;
2223
use ty::{self, TyCtxt, Binder, TypeFoldable};
2324
use ty::error::TypeError;
2425
use ty::relate::{Relate, RelateResult, TypeRelation};
@@ -246,7 +247,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
246247
snapshot: &CombinedSnapshot,
247248
debruijn: ty::DebruijnIndex,
248249
new_vars: &[ty::RegionVid],
249-
a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
250+
a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
250251
r0: ty::Region<'tcx>)
251252
-> ty::Region<'tcx> {
252253
// Regions that pre-dated the LUB computation stay as they are.
@@ -342,7 +343,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
342343
snapshot: &CombinedSnapshot,
343344
debruijn: ty::DebruijnIndex,
344345
new_vars: &[ty::RegionVid],
345-
a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
346+
a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
346347
a_vars: &[ty::RegionVid],
347348
b_vars: &[ty::RegionVid],
348349
r0: ty::Region<'tcx>)
@@ -411,7 +412,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
411412

412413
fn rev_lookup<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
413414
span: Span,
414-
a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
415+
a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
415416
r: ty::Region<'tcx>) -> ty::Region<'tcx>
416417
{
417418
for (a_br, a_r) in a_map {
@@ -434,7 +435,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
434435
}
435436

436437
fn var_ids<'a, 'gcx, 'tcx>(fields: &CombineFields<'a, 'gcx, 'tcx>,
437-
map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
438+
map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
438439
-> Vec<ty::RegionVid> {
439440
map.iter()
440441
.map(|(_, &r)| match *r {

Diff for: ‎src/librustc/infer/lub.rs

+27-1
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
/// "Least upper bound" (common supertype)
@@ -74,7 +75,32 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
7475
-> RelateResult<'tcx, ty::Binder<T>>
7576
where T: Relate<'tcx>
7677
{
77-
self.fields.higher_ranked_lub(a, b, self.a_is_expected)
78+
debug!("binders(a={:?}, b={:?})", a, b);
79+
let was_error = self.infcx().probe(|_snapshot| {
80+
// Subtle: use a fresh combine-fields here because we recover
81+
// from Err. Doing otherwise could propagate obligations out
82+
// through our `self.obligations` field.
83+
self.infcx()
84+
.combine_fields(self.fields.trace.clone(), self.fields.param_env)
85+
.higher_ranked_lub(a, b, self.a_is_expected)
86+
.is_err()
87+
});
88+
debug!("binders: was_error={:?}", was_error);
89+
90+
// When higher-ranked types are involved, computing the LUB is
91+
// very challenging, switch to invariance. This is obviously
92+
// overly conservative but works ok in practice.
93+
match self.relate_with_variance(ty::Variance::Invariant, a, b) {
94+
Ok(_) => Ok(a.clone()),
95+
Err(err) => {
96+
debug!("binders: error occurred, was_error={:?}", was_error);
97+
if !was_error {
98+
Err(TypeError::OldStyleLUB(Box::new(err)))
99+
} else {
100+
Err(err)
101+
}
102+
}
103+
}
78104
}
79105
}
80106

Diff for: ‎src/librustc/infer/mod.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use ty::relate::RelateResult;
3131
use traits::{self, ObligationCause, PredicateObligations, Reveal};
3232
use rustc_data_structures::unify::{self, UnificationTable};
3333
use std::cell::{Cell, RefCell, Ref, RefMut};
34+
use std::collections::BTreeMap;
3435
use std::fmt;
3536
use syntax::ast;
3637
use errors::DiagnosticBuilder;
@@ -184,7 +185,7 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
184185

185186
/// A map returned by `skolemize_late_bound_regions()` indicating the skolemized
186187
/// region that each late-bound region was replaced with.
187-
pub type SkolemizationMap<'tcx> = FxHashMap<ty::BoundRegion, ty::Region<'tcx>>;
188+
pub type SkolemizationMap<'tcx> = BTreeMap<ty::BoundRegion, ty::Region<'tcx>>;
188189

189190
/// See `error_reporting` module for more details
190191
#[derive(Clone, Debug)]
@@ -1384,7 +1385,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
13841385
span: Span,
13851386
lbrct: LateBoundRegionConversionTime,
13861387
value: &ty::Binder<T>)
1387-
-> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
1388+
-> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
13881389
where T : TypeFoldable<'tcx>
13891390
{
13901391
self.tcx.replace_late_bound_regions(

Diff for: ‎src/librustc/ty/error.rs

+11
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
}
@@ -293,6 +298,12 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
293298
db.span_note(found.origin_span,
294299
"...that also applies to the same type variable here");
295300
}
301+
OldStyleLUB(err) => {
302+
db.note("this was previously accepted by the compiler but has been phased out");
303+
db.note("for more information, see https://github.com/rust-lang/rust/issues/45852");
304+
305+
self.note_and_explain_type_err(db, &err, sp);
306+
}
296307
_ => {}
297308
}
298309
}

Diff for: ‎src/librustc/ty/fold.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ use middle::const_val::ConstVal;
4343
use ty::{self, Binder, Ty, TyCtxt, TypeFlags};
4444

4545
use std::fmt;
46-
use util::nodemap::{FxHashMap, FxHashSet};
46+
use std::collections::BTreeMap;
47+
use util::nodemap::FxHashSet;
4748

4849
/// The TypeFoldable trait is implemented for every type that can be folded.
4950
/// Basically, every type that has a corresponding method in TypeFolder.
@@ -324,14 +325,14 @@ struct RegionReplacer<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
324325
tcx: TyCtxt<'a, 'gcx, 'tcx>,
325326
current_depth: u32,
326327
fld_r: &'a mut (FnMut(ty::BoundRegion) -> ty::Region<'tcx> + 'a),
327-
map: FxHashMap<ty::BoundRegion, ty::Region<'tcx>>
328+
map: BTreeMap<ty::BoundRegion, ty::Region<'tcx>>
328329
}
329330

330331
impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
331332
pub fn replace_late_bound_regions<T,F>(self,
332333
value: &Binder<T>,
333334
mut f: F)
334-
-> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
335+
-> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
335336
where F : FnMut(ty::BoundRegion) -> ty::Region<'tcx>,
336337
T : TypeFoldable<'tcx>,
337338
{
@@ -438,7 +439,7 @@ impl<'a, 'gcx, 'tcx> RegionReplacer<'a, 'gcx, 'tcx> {
438439
tcx,
439440
current_depth: 1,
440441
fld_r,
441-
map: FxHashMap()
442+
map: BTreeMap::default()
442443
}
443444
}
444445
}

Diff for: ‎src/librustc/ty/structural_impls.rs

+4-1
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)