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

Author: bors

src/librustc/infer/error_reporting/mod.rs

@@ -762,16 +762,23 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             }
         }
 
-        self.note_error_origin(diag, &cause);
         self.check_and_note_conflicting_crates(diag, terr, span);
         self.tcx.note_and_explain_type_err(diag, terr, span);
+
+        // It reads better to have the error origin as the final
+        // thing.
+        self.note_error_origin(diag, &cause);
     }
 
     pub fn report_and_explain_type_error(&self,
                                          trace: TypeTrace<'tcx>,
                                          terr: &TypeError<'tcx>)
                                          -> DiagnosticBuilder<'tcx>
     {
+        debug!("report_and_explain_type_error(trace={:?}, terr={:?})",
+               trace,
+               terr);
+
         let span = trace.cause.span;
         let failure_str = trace.cause.as_failure_str();
         let mut diag = match trace.cause.code {

src/librustc/infer/glb.rs

@@ -15,6 +15,7 @@ use super::Subtype;
 
 use traits::ObligationCause;
 use ty::{self, Ty, TyCtxt};
+use ty::error::TypeError;
 use ty::relate::{Relate, RelateResult, TypeRelation};
 
 /// "Greatest lower bound" (common subtype)
@@ -74,7 +75,32 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
                   -> RelateResult<'tcx, ty::Binder<T>>
         where T: Relate<'tcx>
     {
-        self.fields.higher_ranked_glb(a, b, self.a_is_expected)
+        debug!("binders(a={:?}, b={:?})", a, b);
+        let was_error = self.infcx().probe(|_snapshot| {
+            // Subtle: use a fresh combine-fields here because we recover
+            // from Err. Doing otherwise could propagate obligations out
+            // through our `self.obligations` field.
+            self.infcx()
+                .combine_fields(self.fields.trace.clone(), self.fields.param_env)
+                .higher_ranked_glb(a, b, self.a_is_expected)
+                .is_err()
+        });
+        debug!("binders: was_error={:?}", was_error);
+
+        // When higher-ranked types are involved, computing the LUB is
+        // very challenging, switch to invariance. This is obviously
+        // overly conservative but works ok in practice.
+        match self.relate_with_variance(ty::Variance::Invariant, a, b) {
+            Ok(_) => Ok(a.clone()),
+            Err(err) => {
+                debug!("binders: error occurred, was_error={:?}", was_error);
+                if !was_error {
+                    Err(TypeError::OldStyleLUB(Box::new(err)))
+                } else {
+                    Err(err)
+                }
+            }
+        }
     }
 }
 

src/librustc/infer/higher_ranked/mod.rs

@@ -19,6 +19,7 @@ use super::{CombinedSnapshot,
 use super::combine::CombineFields;
 use super::region_constraints::{TaintDirections};
 
+use std::collections::BTreeMap;
 use ty::{self, TyCtxt, Binder, TypeFoldable};
 use ty::error::TypeError;
 use ty::relate::{Relate, RelateResult, TypeRelation};
@@ -246,7 +247,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
                                              snapshot: &CombinedSnapshot,
                                              debruijn: ty::DebruijnIndex,
                                              new_vars: &[ty::RegionVid],
-                                             a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
+                                             a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
                                              r0: ty::Region<'tcx>)
                                              -> ty::Region<'tcx> {
             // Regions that pre-dated the LUB computation stay as they are.
@@ -342,7 +343,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
                                              snapshot: &CombinedSnapshot,
                                              debruijn: ty::DebruijnIndex,
                                              new_vars: &[ty::RegionVid],
-                                             a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
+                                             a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
                                              a_vars: &[ty::RegionVid],
                                              b_vars: &[ty::RegionVid],
                                              r0: ty::Region<'tcx>)
@@ -411,7 +412,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
 
         fn rev_lookup<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
                                       span: Span,
-                                      a_map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>,
+                                      a_map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>,
                                       r: ty::Region<'tcx>) -> ty::Region<'tcx>
         {
             for (a_br, a_r) in a_map {
@@ -434,7 +435,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
 }
 
 fn var_ids<'a, 'gcx, 'tcx>(fields: &CombineFields<'a, 'gcx, 'tcx>,
-                           map: &FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
+                           map: &BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
                            -> Vec<ty::RegionVid> {
     map.iter()
        .map(|(_, &r)| match *r {

src/librustc/infer/lub.rs

@@ -15,6 +15,7 @@ use super::Subtype;
 
 use traits::ObligationCause;
 use ty::{self, Ty, TyCtxt};
+use ty::error::TypeError;
 use ty::relate::{Relate, RelateResult, TypeRelation};
 
 /// "Least upper bound" (common supertype)
@@ -74,7 +75,32 @@ impl<'combine, 'infcx, 'gcx, 'tcx> TypeRelation<'infcx, 'gcx, 'tcx>
                   -> RelateResult<'tcx, ty::Binder<T>>
         where T: Relate<'tcx>
     {
-        self.fields.higher_ranked_lub(a, b, self.a_is_expected)
+        debug!("binders(a={:?}, b={:?})", a, b);
+        let was_error = self.infcx().probe(|_snapshot| {
+            // Subtle: use a fresh combine-fields here because we recover
+            // from Err. Doing otherwise could propagate obligations out
+            // through our `self.obligations` field.
+            self.infcx()
+                .combine_fields(self.fields.trace.clone(), self.fields.param_env)
+                .higher_ranked_lub(a, b, self.a_is_expected)
+                .is_err()
+        });
+        debug!("binders: was_error={:?}", was_error);
+
+        // When higher-ranked types are involved, computing the LUB is
+        // very challenging, switch to invariance. This is obviously
+        // overly conservative but works ok in practice.
+        match self.relate_with_variance(ty::Variance::Invariant, a, b) {
+            Ok(_) => Ok(a.clone()),
+            Err(err) => {
+                debug!("binders: error occurred, was_error={:?}", was_error);
+                if !was_error {
+                    Err(TypeError::OldStyleLUB(Box::new(err)))
+                } else {
+                    Err(err)
+                }
+            }
+        }
     }
 }
 

src/librustc/infer/mod.rs

@@ -31,6 +31,7 @@ use ty::relate::RelateResult;
 use traits::{self, ObligationCause, PredicateObligations, Reveal};
 use rustc_data_structures::unify::{self, UnificationTable};
 use std::cell::{Cell, RefCell, Ref, RefMut};
+use std::collections::BTreeMap;
 use std::fmt;
 use syntax::ast;
 use errors::DiagnosticBuilder;
@@ -184,7 +185,7 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
 
 /// A map returned by `skolemize_late_bound_regions()` indicating the skolemized
 /// region that each late-bound region was replaced with.
-pub type SkolemizationMap<'tcx> = FxHashMap<ty::BoundRegion, ty::Region<'tcx>>;
+pub type SkolemizationMap<'tcx> = BTreeMap<ty::BoundRegion, ty::Region<'tcx>>;
 
 /// See `error_reporting` module for more details
 #[derive(Clone, Debug)]
@@ -1384,7 +1385,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         span: Span,
         lbrct: LateBoundRegionConversionTime,
         value: &ty::Binder<T>)
-        -> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
+        -> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
         where T : TypeFoldable<'tcx>
     {
         self.tcx.replace_late_bound_regions(

src/librustc/ty/error.rs

@@ -54,6 +54,8 @@ pub enum TypeError<'tcx> {
     ProjectionBoundsLength(ExpectedFound<usize>),
     TyParamDefaultMismatch(ExpectedFound<type_variable::Default<'tcx>>),
     ExistentialMismatch(ExpectedFound<&'tcx ty::Slice<ty::ExistentialPredicate<'tcx>>>),
+
+    OldStyleLUB(Box<TypeError<'tcx>>),
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Hash, Debug, Copy)]
@@ -170,6 +172,9 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
                 report_maybe_different(f, format!("trait `{}`", values.expected),
                                        format!("trait `{}`", values.found))
             }
+            OldStyleLUB(ref err) => {
+                write!(f, "{}", err)
+            }
         }
     }
 }
@@ -293,6 +298,12 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
                 db.span_note(found.origin_span,
                              "...that also applies to the same type variable here");
             }
+            OldStyleLUB(err) => {
+                db.note("this was previously accepted by the compiler but has been phased out");
+                db.note("for more information, see https://github.com/rust-lang/rust/issues/45852");
+
+                self.note_and_explain_type_err(db, &err, sp);
+            }
             _ => {}
         }
     }

src/librustc/ty/fold.rs

@@ -43,7 +43,8 @@ use middle::const_val::ConstVal;
 use ty::{self, Binder, Ty, TyCtxt, TypeFlags};
 
 use std::fmt;
-use util::nodemap::{FxHashMap, FxHashSet};
+use std::collections::BTreeMap;
+use util::nodemap::FxHashSet;
 
 /// The TypeFoldable trait is implemented for every type that can be folded.
 /// Basically, every type that has a corresponding method in TypeFolder.
@@ -324,14 +325,14 @@ struct RegionReplacer<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'gcx, 'tcx>,
     current_depth: u32,
     fld_r: &'a mut (FnMut(ty::BoundRegion) -> ty::Region<'tcx> + 'a),
-    map: FxHashMap<ty::BoundRegion, ty::Region<'tcx>>
+    map: BTreeMap<ty::BoundRegion, ty::Region<'tcx>>
 }
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
     pub fn replace_late_bound_regions<T,F>(self,
         value: &Binder<T>,
         mut f: F)
-        -> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
+        -> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
         where F : FnMut(ty::BoundRegion) -> ty::Region<'tcx>,
               T : TypeFoldable<'tcx>,
     {
@@ -438,7 +439,7 @@ impl<'a, 'gcx, 'tcx> RegionReplacer<'a, 'gcx, 'tcx> {
             tcx,
             current_depth: 1,
             fld_r,
-            map: FxHashMap()
+            map: BTreeMap::default()
         }
     }
 }

src/librustc/ty/structural_impls.rs

@@ -428,7 +428,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
             TyParamDefaultMismatch(ref x) => {
                 return tcx.lift(x).map(TyParamDefaultMismatch)
             }
-            ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch)
+            ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch),
+            OldStyleLUB(ref x) => return tcx.lift(x).map(OldStyleLUB),
         })
     }
 }
@@ -1174,6 +1175,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::error::TypeError<'tcx> {
             Sorts(x) => Sorts(x.fold_with(folder)),
             TyParamDefaultMismatch(ref x) => TyParamDefaultMismatch(x.fold_with(folder)),
             ExistentialMismatch(x) => ExistentialMismatch(x.fold_with(folder)),
+            OldStyleLUB(ref x) => OldStyleLUB(x.fold_with(folder)),
         }
     }
 
@@ -1191,6 +1193,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::error::TypeError<'tcx> {
                 b.visit_with(visitor)
             },
             Sorts(x) => x.visit_with(visitor),
+            OldStyleLUB(ref x) => x.visit_with(visitor),
             TyParamDefaultMismatch(ref x) => x.visit_with(visitor),
             ExistentialMismatch(x) => x.visit_with(visitor),
             Mismatch |