introduce new fallback algorithm

We now fallback type variables using the following rules:

* Construct a coercion graph `A -> B` where `A` and `B` are unresolved
  type variables or the `!` type.
* Let D be those variables that are reachable from `!`.
* Let N be those variables that are reachable from a variable not in
D.
* All variables in (D \ N) fallback to `!`.
* All variables in (D & N) fallback to `()`.

compiler/rustc_infer/src/infer/mod.rs

@@ -707,11 +707,17 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     /// No attempt is made to resolve `ty`.
     pub fn type_var_diverges(&'a self, ty: Ty<'_>) -> Diverging {
         match *ty.kind() {
-            ty::Infer(ty::TyVar(vid)) => self.inner.borrow_mut().type_variables().var_diverges(vid),
+            ty::Infer(ty::TyVar(vid)) => self.ty_vid_diverges(vid),
             _ => Diverging::NotDiverging,
         }
     }
 
+    /// Returns true if the type inference variable `vid` was created
+    /// as a diverging type variable. No attempt is made to resolve `vid`.
+    pub fn ty_vid_diverges(&'a self, vid: ty::TyVid) -> Diverging {
+        self.inner.borrow_mut().type_variables().var_diverges(vid)
+    }
+
     /// Returns the origin of the type variable identified by `vid`, or `None`
     /// if this is not a type variable.
     ///
@@ -1070,6 +1076,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         })
     }
 
+    /// Number of type variables created so far.
+    pub fn num_ty_vars(&self) -> usize {
+        self.inner.borrow_mut().type_variables().num_vars()
+    }
+
     pub fn next_ty_var_id(&self, diverging: Diverging, origin: TypeVariableOrigin) -> TyVid {
         self.inner.borrow_mut().type_variables().new_var(self.universe(), diverging, origin)
     }

compiler/rustc_middle/src/ty/sty.rs

@@ -1672,6 +1672,14 @@ impl<'tcx> TyS<'tcx> {
         matches!(self.kind(), Infer(TyVar(_)))
     }
 
+    #[inline]
+    pub fn ty_vid(&self) -> Option<ty::TyVid> {
+        match self.kind() {
+            &Infer(TyVar(vid)) => Some(vid),
+            _ => None,
+        }
+    }
+
     #[inline]
     pub fn is_ty_infer(&self) -> bool {
         matches!(self.kind(), Infer(_))

compiler/rustc_typeck/src/check/fallback.rs

@@ -1,4 +1,7 @@
 use crate::check::FnCtxt;
+use rustc_data_structures::{
+    fx::FxHashMap, graph::vec_graph::VecGraph, graph::WithSuccessors, stable_set::FxHashSet,
+};
 use rustc_infer::infer::type_variable::Diverging;
 use rustc_middle::ty::{self, Ty};
 
@@ -8,22 +11,30 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
     pub(super) fn type_inference_fallback(&self) -> bool {
         // All type checking constraints were added, try to fallback unsolved variables.
         self.select_obligations_where_possible(false, |_| {});
-        let mut fallback_has_occurred = false;
 
+        // 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 false;
+        }
+
+        let diverging_fallback = self.calculate_diverging_fallback(&unsolved_variables);
+
+        let mut fallback_has_occurred = false;
         // We do fallback in two passes, to try to generate
         // better error messages.
         // The first time, we do *not* replace opaque types.
-        for ty in &self.unsolved_variables() {
+        for ty in unsolved_variables {
             debug!("unsolved_variable = {:?}", ty);
-            fallback_has_occurred |= self.fallback_if_possible(ty);
+            fallback_has_occurred |= self.fallback_if_possible(ty, &diverging_fallback);
         }
 
-        // We now see if we can make progress. This might
+        // We now see if we can make progress. This might cause us to
-        // cause us to unify inference variables for opaque types,
+        // unify inference variables for opaque types, since we may
-        // since we may have unified some other type variables
+        // have unified some other type variables during the first
-        // during the first phase of fallback.
+        // phase of fallback.  This means that we only replace
-        // This means that we only replace inference variables with their underlying
+        // inference variables with their underlying opaque types as a
-        // opaque types as a last resort.
+        // last resort.
         //
         // In code like this:
         //
@@ -62,36 +73,44 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
     //
     // - Unconstrained floats are replaced with with `f64`.
     //
-    // - Non-numerics get replaced with `!` when `#![feature(never_type_fallback)]`
+    // - Non-numerics may get replaced with `()` or `!`, depending on
-    //   is enabled. Otherwise, they are replaced with `()`.
+    //   how they were categorized by `calculate_diverging_fallback`
+    //   (and the setting of `#![feature(never_type_fallback)]`).
+    //
+    // Fallback becomes very dubious if we have encountered
+    // type-checking errors.  In that case, fallback to Error.
     //
-    // Fallback becomes very dubious if we have encountered type-checking errors.
-    // In that case, fallback to Error.
     // The return value indicates whether fallback has occurred.
-    fn fallback_if_possible(&self, ty: Ty<'tcx>) -> bool {
+    fn fallback_if_possible(
+        &self,
+        ty: Ty<'tcx>,
+        diverging_fallback: &FxHashMap<Ty<'tcx>, Ty<'tcx>>,
+    ) -> bool {
         // Careful: we do NOT shallow-resolve `ty`. We know that `ty`
-        // is an unsolved variable, and we determine its fallback based
+        // is an unsolved variable, and we determine its fallback
-        // solely on how it was created, not what other type variables
+        // based solely on how it was created, not what other type
-        // it may have been unified with since then.
+        // variables it may have been unified with since then.
         //
-        // The reason this matters is that other attempts at fallback may
+        // The reason this matters is that other attempts at fallback
-        // (in principle) conflict with this fallback, and we wish to generate
+        // may (in principle) conflict with this fallback, and we wish
-        // a type error in that case. (However, this actually isn't true right now,
+        // to generate a type error in that case. (However, this
-        // because we're only using the builtin fallback rules. This would be
+        // actually isn't true right now, because we're only using the
-        // true if we were using user-supplied fallbacks. But it's still useful
+        // builtin fallback rules. This would be true if we were using
-        // to write the code to detect bugs.)
+        // user-supplied fallbacks. But it's still useful to write the
+        // code to detect bugs.)
         //
-        // (Note though that if we have a general type variable `?T` that is then unified
+        // (Note though that if we have a general type variable `?T`
-        // with an integer type variable `?I` that ultimately never gets
+        // that is then unified with an integer type variable `?I`
-        // resolved to a special integral type, `?T` is not considered unsolved,
+        // that ultimately never gets resolved to a special integral
-        // but `?I` is. The same is true for float variables.)
+        // type, `?T` is not considered unsolved, but `?I` is. The
+        // same is true for float variables.)
         let fallback = match ty.kind() {
             _ if self.is_tainted_by_errors() => self.tcx.ty_error(),
             ty::Infer(ty::IntVar(_)) => self.tcx.types.i32,
             ty::Infer(ty::FloatVar(_)) => self.tcx.types.f64,
-            _ => match self.type_var_diverges(ty) {
+            _ => match diverging_fallback.get(&ty) {
-                Diverging::Diverges => self.tcx.mk_diverging_default(),
+                Some(&fallback_ty) => fallback_ty,
-                Diverging::NotDiverging => return false,
+                None => return false,
             },
         };
         debug!("fallback_if_possible(ty={:?}): defaulting to `{:?}`", ty, fallback);
@@ -105,11 +124,10 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
         true
     }
 
-    /// Second round of fallback: Unconstrained type variables
+    /// Second round of fallback: Unconstrained type variables created
-    /// created from the instantiation of an opaque
+    /// from the instantiation of an opaque type fall back to the
-    /// type fall back to the opaque type itself. This is a
+    /// opaque type itself. This is a somewhat incomplete attempt to
-    /// somewhat incomplete attempt to manage "identity passthrough"
+    /// manage "identity passthrough" for `impl Trait` types.
-    /// for `impl Trait` types.
     ///
     /// For example, in this code:
     ///
@@ -158,4 +176,195 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
             return false;
         }
     }
+
+    /// The "diverging fallback" system is rather complicated. This is
+    /// a result of our need to balance 'do the right thing' with
+    /// backwards compatibility.
+    ///
+    /// "Diverging" type variables are variables created when we
+    /// coerce a `!` type into an unbound type variable `?X`. If they
+    /// never wind up being constrained, the "right and natural" thing
+    /// is that `?X` should "fallback" to `!`. This means that e.g. an
+    /// expression like `Some(return)` will ultimately wind up with a
+    /// type like `Option<!>` (presuming it is not assigned or
+    /// constrained to have some other type).
+    ///
+    /// However, the fallback used to be `()` (before the `!` type was
+    /// added).  Moreover, there are cases where the `!` type 'leaks
+    /// out' from dead code into type variables that affect live
+    /// code. The most common case is something like this:
+    ///
+    /// ```rust
+    /// match foo() {
+    ///     22 => Default::default(), // call this type `?D`
+    ///     _ => return, // return has type `!`
+    /// } // call the type of this match `?M`
+    /// ```
+    ///
+    /// Here, coercing the type `!` into `?M` will create a diverging
+    /// type variable `?X` where `?X <: ?M`.  We also have that `?D <:
+    /// ?M`. If `?M` winds up unconstrained, then `?X` will
+    /// fallback. If it falls back to `!`, then all the type variables
+    /// will wind up equal to `!` -- this includes the type `?D`
+    /// (since `!` doesn't implement `Default`, we wind up a "trait
+    /// not implemented" error in code like this). But since the
+    /// original fallback was `()`, this code used to compile with `?D
+    /// = ()`. This is somewhat surprising, since `Default::default()`
+    /// on its own would give an error because the types are
+    /// insufficiently constrained.
+    ///
+    /// Our solution to this dilemma is to modify diverging variables
+    /// so that they can *either* fallback to `!` (the default) or to
+    /// `()` (the backwards compatibility case). We decide which
+    /// fallback to use based on whether there is a coercion pattern
+    /// like this:
+    ///
+    /// ```
+    /// ?Diverging -> ?V
+    /// ?NonDiverging -> ?V
+    /// ?V != ?NonDiverging
+    /// ```
+    ///
+    /// Here `?Diverging` represents some diverging type variable and
+    /// `?NonDiverging` represents some non-diverging type
+    /// variable. `?V` can be any type variable (diverging or not), so
+    /// long as it is not equal to `?NonDiverging`.
+    ///
+    /// Intuitively, what we are looking for is a case where a
+    /// "non-diverging" type variable (like `?M` in our example above)
+    /// is coerced *into* some variable `?V` that would otherwise
+    /// fallback to `!`. In that case, we make `?V` fallback to `!`,
+    /// along with anything that would flow into `?V`.
+    ///
+    /// The algorithm we use:
+    /// * Identify all variables that are coerced *into* by a
+    ///   diverging variable.  Do this by iterating over each
+    ///   diverging, unsolved variable and finding all variables
+    ///   reachable from there. Call that set `D`.
+    /// * Walk over all unsolved, non-diverging variables, and find
+    ///   any variable that has an edge into `D`.
+    fn calculate_diverging_fallback(
+        &self,
+        unsolved_variables: &[Ty<'tcx>],
+    ) -> FxHashMap<Ty<'tcx>, Ty<'tcx>> {
+        debug!("calculate_diverging_fallback({:?})", unsolved_variables);
+
+        // Construct a coercion graph where an edge `A -> B` indicates
+        // a type variable is that is coerced
+        let coercion_graph = self.create_coercion_graph();
+
+        // Extract the unsolved type inference variable vids; note that some
+        // unsolved variables are integer/float variables and are excluded.
+        let unsolved_vids: Vec<_> =
+            unsolved_variables.iter().filter_map(|ty| ty.ty_vid()).collect();
+
+        // Find all type variables that are reachable from a diverging
+        // type variable. These will typically default to `!`, unless
+        // we find later that they are *also* reachable from some
+        // other type variable outside this set.
+        let mut roots_reachable_from_diverging = FxHashSet::default();
+        let mut diverging_vids = vec![];
+        let mut non_diverging_vids = vec![];
+        for &unsolved_vid in &unsolved_vids {
+            debug!(
+                "calculate_diverging_fallback: unsolved_vid={:?} diverges={:?}",
+                unsolved_vid,
+                self.infcx.ty_vid_diverges(unsolved_vid)
+            );
+            match self.infcx.ty_vid_diverges(unsolved_vid) {
+                Diverging::Diverges => {
+                    diverging_vids.push(unsolved_vid);
+                    let root_vid = self.infcx.root_var(unsolved_vid);
+                    debug!(
+                        "calculate_diverging_fallback: root_vid={:?} reaches {:?}",
+                        root_vid,
+                        coercion_graph.depth_first_search(root_vid).collect::<Vec<_>>()
+                    );
+                    roots_reachable_from_diverging
+                        .extend(coercion_graph.depth_first_search(root_vid));
+                }
+                Diverging::NotDiverging => {
+                    non_diverging_vids.push(unsolved_vid);
+                }
+            }
+        }
+        debug!(
+            "calculate_diverging_fallback: roots_reachable_from_diverging={:?}",
+            roots_reachable_from_diverging,
+        );
+
+        // Find all type variables N0 that are not reachable from a
+        // diverging variable, and then compute the set reachable from
+        // N0, which we call N. These are the *non-diverging* type
+        // variables. (Note that this set consists of "root variables".)
+        let mut roots_reachable_from_non_diverging = FxHashSet::default();
+        for &non_diverging_vid in &non_diverging_vids {
+            let root_vid = self.infcx.root_var(non_diverging_vid);
+            if roots_reachable_from_diverging.contains(&root_vid) {
+                continue;
+            }
+            roots_reachable_from_non_diverging.extend(coercion_graph.depth_first_search(root_vid));
+        }
+        debug!(
+            "calculate_diverging_fallback: roots_reachable_from_non_diverging={:?}",
+            roots_reachable_from_non_diverging,
+        );
+
+        // For each diverging variable, figure out whether it can
+        // reach a member of N. If so, it falls back to `()`. Else
+        // `!`.
+        let mut diverging_fallback = FxHashMap::default();
+        for &diverging_vid in &diverging_vids {
+            let diverging_ty = self.tcx.mk_ty_var(diverging_vid);
+            let root_vid = self.infcx.root_var(diverging_vid);
+            let can_reach_non_diverging = coercion_graph
+                .depth_first_search(root_vid)
+                .any(|n| roots_reachable_from_non_diverging.contains(&n));
+            if can_reach_non_diverging {
+                debug!("fallback to (): {:?}", diverging_vid);
+                diverging_fallback.insert(diverging_ty, self.tcx.types.unit);
+            } else {
+                debug!("fallback to !: {:?}", diverging_vid);
+                diverging_fallback.insert(diverging_ty, self.tcx.mk_diverging_default());
+            }
+        }
+
+        diverging_fallback
+    }
+
+    /// Returns a graph whose nodes are (unresolved) inference variables and where
+    /// an edge `?A -> ?B` indicates that the variable `?A` is coerced to `?B`.
+    fn create_coercion_graph(&self) -> VecGraph<ty::TyVid> {
+        let pending_obligations = self.fulfillment_cx.borrow_mut().pending_obligations();
+        debug!("create_coercion_graph: pending_obligations={:?}", pending_obligations);
+        let coercion_edges: Vec<(ty::TyVid, ty::TyVid)> = pending_obligations
+            .into_iter()
+            .filter_map(|obligation| {
+                // The predicates we are looking for look like `Coerce(?A -> ?B)`.
+                // They will have no bound variables.
+                obligation.predicate.kind().no_bound_vars()
+            })
+            .filter_map(|atom| {
+                if let ty::PredicateKind::Coerce(ty::CoercePredicate { a, b }) = atom {
+                    let a_vid = self.root_vid(a)?;
+                    let b_vid = self.root_vid(b)?;
+                    Some((a_vid, b_vid))
+                } else {
+                    return None;
+                };
+
+                let a_vid = self.root_vid(a)?;
+                let b_vid = self.root_vid(b)?;
+                Some((a_vid, b_vid))
+            })
+            .collect();
+        debug!("create_coercion_graph: coercion_edges={:?}", coercion_edges);
+        let num_ty_vars = self.infcx.num_ty_vars();
+        VecGraph::new(num_ty_vars, coercion_edges)
+    }
+
+    /// If `ty` is an unresolved type variable, returns its root vid.
+    fn root_vid(&self, ty: Ty<'tcx>) -> Option<ty::TyVid> {
+        Some(self.infcx.root_var(self.infcx.shallow_resolve(ty).ty_vid()?))
+    }
 }

src/test/ui/never_type/diverging-fallback-control-flow.rs

@@ -4,27 +4,24 @@
 #![allow(unused_assignments)]
 #![allow(unused_variables)]
 #![allow(unreachable_code)]
-
 // Test various cases where we permit an unconstrained variable
-// to fallback based on control-flow.
+// to fallback based on control-flow. In all of these cases,
-//
+// the type variable winds up being the target of both a `!` coercion
-// These represent current behavior, but are pretty dubious.  I would
+// and a coercion from a non-`!` variable, and hence falls back to `()`.
-// like to revisit these and potentially change them. --nmatsakis
-
 #![feature(never_type, never_type_fallback)]
 
-trait BadDefault {
+trait UnitDefault {
     fn default() -> Self;
 }
 
-impl BadDefault for u32 {
+impl UnitDefault for u32 {
     fn default() -> Self {
         0
     }
 }
 
-impl BadDefault for ! {
+impl UnitDefault for () {
-    fn default() -> ! {
+    fn default() -> () {
         panic!()
     }
 }
@@ -33,7 +30,7 @@ fn assignment() {
     let x;
 
     if true {
-        x = BadDefault::default();
+        x = UnitDefault::default();
     } else {
         x = return;
     }
@@ -45,13 +42,13 @@ fn assignment_rev() {
     if true {
         x = return;
     } else {
-        x = BadDefault::default();
+        x = UnitDefault::default();
     }
 }
 
 fn if_then_else() {
     let _x = if true {
-        BadDefault::default()
+        UnitDefault::default()
     } else {
         return;
     };
@@ -61,19 +58,19 @@ fn if_then_else_rev() {
     let _x = if true {
         return;
     } else {
-        BadDefault::default()
+        UnitDefault::default()
     };
 }
 
 fn match_arm() {
-    let _x = match Ok(BadDefault::default()) {
+    let _x = match Ok(UnitDefault::default()) {
         Ok(v) => v,
         Err(()) => return,
     };
 }
 
 fn match_arm_rev() {
-    let _x = match Ok(BadDefault::default()) {
+    let _x = match Ok(UnitDefault::default()) {
         Err(()) => return,
         Ok(v) => v,
     };
@@ -84,7 +81,7 @@ fn loop_break() {
         if false {
             break return;
         } else {
-            break BadDefault::default();
+            break UnitDefault::default();
         }
     };
 }
@@ -94,7 +91,7 @@ fn loop_break_rev() {
         if false {
             break return;
         } else {
-            break BadDefault::default();
+            break UnitDefault::default();
         }
     };
 }

src/test/ui/never_type/diverging-fallback-no-leak.rs

@@ -0,0 +1,15 @@
+#![feature(never_type_fallback)]
+
+fn make_unit() {}
+
+trait Test {}
+impl Test for i32 {}
+impl Test for () {}
+
+fn unconstrained_arg<T: Test>(_: T) {}
+
+fn main() {
+    // Here the type variable falls back to `!`,
+    // and hence we get a type error:
+    unconstrained_arg(return); //~ ERROR trait bound `!: Test` is not satisfied
+}

src/test/ui/never_type/diverging-fallback-no-leak.stderr

@@ -0,0 +1,18 @@
+error[E0277]: the trait bound `!: Test` is not satisfied
+  --> $DIR/diverging-fallback-no-leak.rs:14:5
+   |
+LL |     unconstrained_arg(return);
+   |     ^^^^^^^^^^^^^^^^^ the trait `Test` is not implemented for `!`
+   |
+   = note: this trait is implemented for `()`.
+   = note: this error might have been caused by changes to Rust's type-inference algorithm (see issue #48950 <https://github.com/rust-lang/rust/issues/48950> for more information).
+   = help: did you intend to use the type `()` here instead?
+note: required by a bound in `unconstrained_arg`
+  --> $DIR/diverging-fallback-no-leak.rs:9:25
+   |
+LL | fn unconstrained_arg<T: Test>(_: T) {}
+   |                         ^^^^ required by this bound in `unconstrained_arg`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/never_type/diverging-fallback-unconstrained-return.rs

@@ -0,0 +1,34 @@
+// Variant of diverging-falllback-control-flow that tests
+// the specific case of a free function with an unconstrained
+// return type. This captures the pattern we saw in the wild
+// in the objc crate, where changing the fallback from `!` to `()`
+// resulted in unsoundness.
+//
+// check-pass
+
+#![feature(never_type_fallback)]
+
+fn make_unit() {}
+
+trait UnitReturn {}
+impl UnitReturn for i32 {}
+impl UnitReturn for () {}
+
+fn unconstrained_return<T: UnitReturn>() -> T {
+    unsafe {
+        let make_unit_fn: fn() = make_unit;
+        let ffi: fn() -> T = std::mem::transmute(make_unit_fn);
+        ffi()
+    }
+}
+
+fn main() {
+    // In Ye Olde Days, the `T` parameter of `unconstrained_return`
+    // winds up "entangled" with the `!` type that results from
+    // `panic!`, and hence falls back to `()`. This is kind of unfortunate
+    // and unexpected. When we introduced the `!` type, the original
+    // idea was to change that fallback to `!`, but that would have resulted
+    // in this code no longer compiling (or worse, in some cases it injected
+    // unsound results).
+    let _ = if true { unconstrained_return() } else { panic!() };
+}