Relate type variables during predicate registration

This avoids select_obligations_where_possible having an effect, which future
proofs the algorithm.

Author: Mark-Simulacrum

compiler/rustc_infer/src/traits/engine.rs

@@ -1,5 +1,6 @@
 use crate::infer::InferCtxt;
 use crate::traits::Obligation;
+use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_middle::ty::{self, ToPredicate, Ty, WithConstness};
@@ -73,6 +74,8 @@ pub trait TraitEngine<'tcx>: 'tcx {
     }
 
     fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>>;
+
+    fn relationships(&mut self) -> &mut FxHashMap<ty::TyVid, ty::FoundRelationships>;
 }
 
 pub trait TraitEngineExt<'tcx> {

compiler/rustc_middle/src/ty/mod.rs

@@ -2084,3 +2084,16 @@ impl<'tcx> fmt::Debug for SymbolName<'tcx> {
         fmt::Display::fmt(&self.name, fmt)
     }
 }
+
+#[derive(Debug, Default, Copy, Clone)]
+pub struct FoundRelationships {
+    /// This is true if we identified that this Ty (`?T`) is found in a `?T: Foo`
+    /// obligation, where:
+    ///
+    ///  * `Foo` is not `Sized`
+    ///  * `(): Foo` may be satisfied
+    pub self_in_trait: bool,
+    /// This is true if we identified that this Ty (`?T`) is found in a `<_ as
+    /// _>::AssocType = ?T`
+    pub output: bool,
+}

compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs

@@ -7,16 +7,21 @@ use crate::traits::{
     ChalkEnvironmentAndGoal, FulfillmentError, FulfillmentErrorCode, ObligationCause,
     PredicateObligation, SelectionError, TraitEngine,
 };
-use rustc_data_structures::fx::FxIndexSet;
+use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
 use rustc_middle::ty::{self, Ty};
 
 pub struct FulfillmentContext<'tcx> {
     obligations: FxIndexSet<PredicateObligation<'tcx>>,
+
+    relationships: FxHashMap<ty::TyVid, ty::FoundRelationships>,
 }
 
 impl FulfillmentContext<'tcx> {
     crate fn new() -> Self {
-        FulfillmentContext { obligations: FxIndexSet::default() }
+        FulfillmentContext {
+            obligations: FxIndexSet::default(),
+            relationships: FxHashMap::default(),
+        }
     }
 }
 
@@ -39,6 +44,8 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
         assert!(!infcx.is_in_snapshot());
         let obligation = infcx.resolve_vars_if_possible(obligation);
 
+        super::relationships::update(self, infcx, &obligation);
+
         self.obligations.insert(obligation);
     }
 
@@ -149,4 +156,8 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
     fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>> {
         self.obligations.iter().cloned().collect()
     }
+
+    fn relationships(&mut self) -> &mut FxHashMap<ty::TyVid, ty::FoundRelationships> {
+        &mut self.relationships
+    }
 }

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -1,4 +1,5 @@
 use crate::infer::{InferCtxt, TyOrConstInferVar};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::obligation_forest::ProcessResult;
 use rustc_data_structures::obligation_forest::{Error, ForestObligation, Outcome};
 use rustc_data_structures::obligation_forest::{ObligationForest, ObligationProcessor};
@@ -53,6 +54,9 @@ pub struct FulfillmentContext<'tcx> {
     // A list of all obligations that have been registered with this
     // fulfillment context.
     predicates: ObligationForest<PendingPredicateObligation<'tcx>>,
+
+    relationships: FxHashMap<ty::TyVid, ty::FoundRelationships>,
+
     // Should this fulfillment context register type-lives-for-region
     // obligations on its parent infcx? In some cases, region
     // obligations are either already known to hold (normalization) or
@@ -97,6 +101,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
     pub fn new() -> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
+            relationships: FxHashMap::default(),
             register_region_obligations: true,
             usable_in_snapshot: false,
         }
@@ -105,6 +110,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
     pub fn new_in_snapshot() -> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
+            relationships: FxHashMap::default(),
             register_region_obligations: true,
             usable_in_snapshot: true,
         }
@@ -113,6 +119,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
     pub fn new_ignoring_regions() -> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
+            relationships: FxHashMap::default(),
             register_region_obligations: false,
             usable_in_snapshot: false,
         }
@@ -210,6 +217,8 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
 
         assert!(!infcx.is_in_snapshot() || self.usable_in_snapshot);
 
+        super::relationships::update(self, infcx, &obligation);
+
         self.predicates
             .register_obligation(PendingPredicateObligation { obligation, stalled_on: vec![] });
     }
@@ -265,6 +274,10 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
     fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>> {
         self.predicates.map_pending_obligations(|o| o.obligation.clone())
     }
+
+    fn relationships(&mut self) -> &mut FxHashMap<ty::TyVid, ty::FoundRelationships> {
+        &mut self.relationships
+    }
 }
 
 struct FulfillProcessor<'a, 'b, 'tcx> {

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -15,6 +15,7 @@ mod object_safety;
 mod on_unimplemented;
 mod project;
 pub mod query;
+pub(crate) mod relationships;
 mod select;
 mod specialize;
 mod structural_match;

compiler/rustc_trait_selection/src/traits/relationships.rs

@@ -0,0 +1,89 @@
+use crate::infer::InferCtxt;
+use crate::traits::query::evaluate_obligation::InferCtxtExt;
+use crate::traits::{ObligationCause, PredicateObligation};
+use rustc_infer::traits::TraitEngine;
+use rustc_middle::ty::{self, ToPredicate, TypeFoldable};
+
+pub(crate) fn update<'tcx, T>(
+    engine: &mut T,
+    infcx: &InferCtxt<'_, 'tcx>,
+    obligation: &PredicateObligation<'tcx>,
+) where
+    T: TraitEngine<'tcx>,
+{
+    if let ty::PredicateKind::Trait(predicate) = obligation.predicate.kind().skip_binder() {
+        if predicate.trait_ref.def_id
+            != infcx.tcx.require_lang_item(rustc_hir::LangItem::Sized, None)
+        {
+            // fixme: copy of mk_trait_obligation_with_new_self_ty
+            let new_self_ty = infcx.tcx.types.unit;
+
+            let trait_ref = ty::TraitRef {
+                substs: infcx.tcx.mk_substs_trait(new_self_ty, &predicate.trait_ref.substs[1..]),
+                ..predicate.trait_ref
+            };
+
+            // Then contstruct a new obligation with Self = () added
+            // to the ParamEnv, and see if it holds.
+            let o = rustc_infer::traits::Obligation::new(
+                ObligationCause::dummy(),
+                obligation.param_env,
+                obligation
+                    .predicate
+                    .kind()
+                    .map_bound(|_| {
+                        ty::PredicateKind::Trait(ty::TraitPredicate {
+                            trait_ref,
+                            constness: predicate.constness,
+                        })
+                    })
+                    .to_predicate(infcx.tcx),
+            );
+            // Don't report overflow errors. Otherwise equivalent to may_hold.
+            if let Ok(result) = infcx.probe(|_| infcx.evaluate_obligation(&o)) {
+                if result.may_apply() {
+                    if let Some(ty) = infcx
+                        .shallow_resolve(predicate.self_ty())
+                        .ty_vid()
+                        .map(|t| infcx.root_var(t))
+                    {
+                        debug!("relationship: {:?}.self_in_trait = true", ty);
+                        engine.relationships().entry(ty).or_default().self_in_trait = true;
+                    } else {
+                        debug!("relationship: did not find TyVid for self ty...");
+                    }
+                }
+            }
+        }
+    }
+
+    if let ty::PredicateKind::Projection(predicate) = obligation.predicate.kind().skip_binder() {
+        // If the projection predicate (Foo::Bar == X) has X as a non-TyVid,
+        // we need to make it into one.
+        if let Some(vid) = predicate.ty.ty_vid() {
+            debug!("relationship: {:?}.output = true", vid);
+            engine.relationships().entry(vid).or_default().output = true;
+        } else {
+            // This will have registered a projection obligation that will hit
+            // the Some(vid) branch above. So we don't need to do anything
+            // further here.
+            debug!(
+                "skipping relationship for obligation {:?} -- would need to normalize",
+                obligation
+            );
+            if !predicate.projection_ty.has_escaping_bound_vars() {
+                // FIXME: We really *should* do this even with escaping bound
+                // vars, but there's not much we can do here. In the worst case
+                // (if this ends up being important) we just don't register a relationship and then end up falling back to !,
+                // which is not terrible.
+
+                //engine.normalize_projection_type(
+                //    infcx,
+                //    obligation.param_env,
+                //    predicate.projection_ty,
+                //    obligation.cause.clone(),
+                //);
+            }
+        }
+    }
+}

compiler/rustc_typeck/src/check/fallback.rs

@@ -5,22 +5,8 @@ use rustc_data_structures::{
     graph::{iterate::DepthFirstSearch, vec_graph::VecGraph},
     stable_set::FxHashSet,
 };
-use rustc_middle::traits;
-use rustc_middle::ty::{self, ToPredicate, Ty};
-use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
-
-#[derive(Default, Copy, Clone)]
-struct FoundRelationships {
-    /// This is true if we identified that this Ty (`?T`) is found in a `?T: Foo`
-    /// obligation, where:
-    ///
-    ///  * `Foo` is not `Sized`
-    ///  * `(): Foo` may be satisfied
-    self_in_trait: bool,
-    /// This is true if we identified that this Ty (`?T`) is found in a `<_ as
-    /// _>::AssocType = ?T`
-    output: bool,
-}
+use rustc_middle::ty::{self, Ty};
+
 impl<'tcx> FnCtxt<'_, 'tcx> {
     /// Performs type inference fallback, returning true if any fallback
     /// occurs.
@@ -30,60 +16,12 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
             self.fulfillment_cx.borrow_mut().pending_obligations()
         );
 
-        let mut relationships: FxHashMap<ty::TyVid, FoundRelationships> = FxHashMap::default();
-        for obligation in self.fulfillment_cx.borrow_mut().pending_obligations() {
-            if let ty::PredicateKind::Trait(predicate, constness) =
-                obligation.predicate.kind().skip_binder()
-            {
-                if predicate.trait_ref.def_id
-                    != self.infcx.tcx.require_lang_item(rustc_hir::LangItem::Sized, None)
-                {
-                    // fixme: copy of mk_trait_obligation_with_new_self_ty
-                    let new_self_ty = self.infcx.tcx.types.unit;
-
-                    let trait_ref = ty::TraitRef {
-                        substs: self
-                            .infcx
-                            .tcx
-                            .mk_substs_trait(new_self_ty, &predicate.trait_ref.substs[1..]),
-                        ..predicate.trait_ref
-                    };
-
-                    // Then contstruct a new obligation with Self = () added
-                    // to the ParamEnv, and see if it holds.
-                    let o = rustc_infer::traits::Obligation::new(
-                        traits::ObligationCause::dummy(),
-                        obligation.param_env,
-                        obligation
-                            .predicate
-                            .kind()
-                            .map_bound(|_| {
-                                ty::PredicateKind::Trait(
-                                    ty::TraitPredicate { trait_ref },
-                                    constness,
-                                )
-                            })
-                            .to_predicate(self.infcx.tcx),
-                    );
-                    if self.infcx.predicate_may_hold(&o) {
-                        if let Some(ty) = self.root_vid(predicate.self_ty()) {
-                            relationships.entry(ty).or_default().self_in_trait = true;
-                        }
-                    }
-                }
-            }
-            if let ty::PredicateKind::Projection(predicate) =
-                obligation.predicate.kind().skip_binder()
-            {
-                if let Some(ty) = self.root_vid(predicate.ty) {
-                    relationships.entry(ty).or_default().output = true;
-                }
-            }
-        }
-
         // All type checking constraints were added, try to fallback unsolved variables.
         self.select_obligations_where_possible(false, |_| {});
 
+        let relationships = self.fulfillment_cx.borrow_mut().relationships().clone();
+
+        debug!("relationships: {:#?}", relationships);
         debug!(
             "type-inference-fallback post selection obligations: {:#?}",
             self.fulfillment_cx.borrow_mut().pending_obligations()
@@ -94,7 +32,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
         // Check if we have any unsolved varibales. If not, no need for fallback.
         let unsolved_variables = self.unsolved_variables();
         if unsolved_variables.is_empty() {
-            return;
+            return false;
         }
 
         let diverging_fallback =
@@ -324,7 +262,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
     fn calculate_diverging_fallback(
         &self,
         unsolved_variables: &[Ty<'tcx>],
-        relationships: &FxHashMap<ty::TyVid, FoundRelationships>,
+        relationships: &FxHashMap<ty::TyVid, ty::FoundRelationships>,
     ) -> FxHashMap<Ty<'tcx>, Ty<'tcx>> {
         debug!("calculate_diverging_fallback({:?})", unsolved_variables);
 
@@ -413,6 +351,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
 
         debug!("inherited: {:#?}", self.inh.fulfillment_cx.borrow_mut().pending_obligations());
         debug!("obligations: {:#?}", self.fulfillment_cx.borrow_mut().pending_obligations());
+        debug!("relationships: {:#?}", relationships);
 
         // For each diverging variable, figure out whether it can
         // reach a member of N. If so, it falls back to `()`. Else
@@ -426,7 +365,15 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
                 .depth_first_search(root_vid)
                 .any(|n| roots_reachable_from_non_diverging.visited(n));
 
-            let relationship = relationships.get(&root_vid).copied().unwrap_or_default();
+            let mut relationship = ty::FoundRelationships { self_in_trait: false, output: false };
+
+            for (vid, rel) in relationships.iter() {
+                //if self.infcx.shallow_resolve(*ty).ty_vid().map(|t| self.infcx.root_var(t))
+                if self.infcx.root_var(*vid) == root_vid {
+                    relationship.self_in_trait |= rel.self_in_trait;
+                    relationship.output |= rel.output;
+                }
+            }
 
             if relationship.self_in_trait && relationship.output {
                 debug!("fallback to () - found trait and projection: {:?}", diverging_vid);

src/test/ui/never_type/fallback-closure-wrap.rs

@@ -0,0 +1,16 @@
+use std::marker::PhantomData;
+
+fn main() {
+    let error = Closure::wrap(Box::new(move || {
+    //~^ ERROR type mismatch
+        panic!("Can't connect to server.");
+    }) as Box<dyn FnMut()>);
+}
+
+struct Closure<T: ?Sized>(PhantomData<T>);
+
+impl<T: ?Sized> Closure<T> {
+    fn wrap(data: Box<T>) -> Closure<T> {
+        todo!()
+    }
+}

src/test/ui/never_type/fallback-closure-wrap.stderr

@@ -0,0 +1,17 @@
+error[E0271]: type mismatch resolving `<[closure@$DIR/fallback-closure-wrap.rs:4:40: 7:6] as FnOnce<()>>::Output == ()`
+  --> $DIR/fallback-closure-wrap.rs:4:31
+   |
+LL |       let error = Closure::wrap(Box::new(move || {
+   |  _______________________________^
+LL | |
+LL | |         panic!("Can't connect to server.");
+LL | |     }) as Box<dyn FnMut()>);
+   | |______^ expected `()`, found `!`
+   |
+   = note: expected unit type `()`
+                   found type `!`
+   = note: required for the cast to the object type `dyn FnMut()`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0271`.