bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #88804 - Mark-Simulacrum:never-algo-v2, r=nikomatsakis,jackh726

Browse source 
Revise never type fallback algorithm

This is a rebase of https://github.com/rust-lang/rust/pull/84573, but dropping the stabilization of never type (and the accompanying large test diff).

Each commit builds & has tests updated alongside it, and could be reviewed in a more or less standalone fashion. But it may make more sense to review the PR as a whole, I'm not sure. It should be noted that tests being updated isn't really a good indicator of final behavior -- never_type_fallback is not enabled by default in this PR, so we can't really see the full effects of the commits here.

This combines the work by Niko, which is [documented in this gist](https://gist.github.com/nikomatsakis/7a07b265dc12f5c3b3bd0422018fa660), with some additional rules largely derived to target specific known patterns that regress with the algorithm solely derived by Niko. We build these from an intuition that:

* In general, fallback to `()` is *sound* in all cases
* But, in general, we *prefer* fallback to `!` as it accepts more code, particularly that written to intentionally use `!` (e.g., Result's with a Infallible/! variant).

When evaluating Niko's proposed algorithm, we find that there are certain cases where fallback to `!` leads to compilation failures in real-world code, and fallback to `()` fixes those errors. In order to allow for stabilization, we need to fix a good portion of these patterns.

The final rule set this PR proposes is that, by default, we fallback from `?T` to `!`, with the following exceptions:

1. `?T: Foo` and `Bar::Baz = ?T` and `(): Foo`, then fallback to `()`
2. Per [Niko's algorithm](https://gist.github.com/nikomatsakis/7a07b265dc12f5c3b3bd0422018fa660#proposal-fallback-chooses-between--and--based-on-the-coercion-graph), the "live" `?T` also fallback to `()`.

The first rule is necessary to address a fairly common pattern which boils down to something like the snippet below. Without rule 1, we do not see the closure's return type as needing a () fallback, which leads to compilation failure.

```rust
#![feature(never_type_fallback)]

trait Bar { }
impl Bar for () {  }
impl Bar for u32 {  }

fn foo<R: Bar>(_: impl Fn() -> R) {}

fn main() {
    foo(|| panic!());
}
```

r? `@jackh726`
This commit is contained in:
bors 2021-09-23 22:45:22 +00:00
commit 900cf5e890
30 changed files with 733 additions and 174 deletions
Download patch file Download diff file Expand all files Collapse all files

32
compiler/rustc_typeck/src/check/check.rs
View file

@ -241,32 +241,16 @@ pub(super) fn check_fn<'a, 'tcx>(
// we saw and assigning it to the expected return type. This isn't
// really expected to fail, since the coercions would have failed
// earlier when trying to find a LUB.
//
// However, the behavior around `!` is sort of complex. In the
// event that the `actual_return_ty` comes back as `!`, that
// indicates that the fn either does not return or "returns" only
// values of type `!`. In this case, if there is an expected
// return type that is *not* `!`, that should be ok. But if the
// return type is being inferred, we want to "fallback" to `!`:
//
// let x = move || panic!();
//
// To allow for that, I am creating a type variable with diverging
// fallback. This was deemed ever so slightly better than unifying
// the return value with `!` because it allows for the caller to
// make more assumptions about the return type (e.g., they could do
//
// let y: Option<u32> = Some(x());
//
// which would then cause this return type to become `u32`, not
// `!`).
let coercion = fcx.ret_coercion.take().unwrap().into_inner();
let mut actual_return_ty = coercion.complete(&fcx);
if actual_return_ty.is_never() {
actual_return_ty = fcx.next_diverging_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::DivergingFn,
span,
});
debug!("actual_return_ty = {:?}", actual_return_ty);
if let ty::Dynamic(..) = declared_ret_ty.kind() {
// We have special-cased the case where the function is declared
// `-> dyn Foo` and we don't actually relate it to the
// `fcx.ret_coercion`, so just substitute a type variable.
actual_return_ty =
fcx.next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::DynReturnFn, span });
debug!("actual_return_ty replaced with {:?}", actual_return_ty);
}
fcx.demand_suptype(span, revealed_ret_ty, actual_return_ty);

19
compiler/rustc_typeck/src/check/coercion.rs
View file

@ -159,24 +159,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// Coercing from `!` to any type is allowed:
if a.is_never() {
// Subtle: If we are coercing from `!` to `?T`, where `?T` is an unbound
// type variable, we want `?T` to fallback to `!` if not
// otherwise constrained. An example where this arises:
//
// let _: Option<?T> = Some({ return; });
//
// here, we would coerce from `!` to `?T`.
return if b.is_ty_var() {
// Micro-optimization: no need for this if `b` is
// already resolved in some way.
let diverging_ty = self.next_diverging_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::AdjustmentType,
span: self.cause.span,
});
self.coerce_from_inference_variable(diverging_ty, b, simple(Adjust::NeverToAny))
} else {
success(simple(Adjust::NeverToAny)(b), b, vec![])
};
return success(simple(Adjust::NeverToAny)(b), b, vec![]);
}
// Coercing *from* an unresolved inference variable means that

2
compiler/rustc_typeck/src/check/expr.rs
View file

@ -77,7 +77,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
!self.typeck_results.borrow().adjustments().contains_key(expr.hir_id),
"expression with never type wound up being adjusted"
);
let adj_ty = self.next_diverging_ty_var(TypeVariableOrigin {
let adj_ty = self.next_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::AdjustmentType,
span: expr.span,
});

372
compiler/rustc_typeck/src/check/fallback.rs
View file

@ -1,29 +1,52 @@
use crate::check::FnCtxt;
use rustc_infer::infer::type_variable::Diverging;
use rustc_data_structures::{
fx::FxHashMap,
graph::WithSuccessors,
graph::{iterate::DepthFirstSearch, vec_graph::VecGraph},
stable_set::FxHashSet,
};
use rustc_middle::ty::{self, Ty};
impl<'tcx> FnCtxt<'_, 'tcx> {
/// Performs type inference fallback, returning true if any fallback
/// occurs.
pub(super) fn type_inference_fallback(&self) -> bool {
debug!(
"type-inference-fallback start obligations: {:#?}",
self.fulfillment_cx.borrow_mut().pending_obligations()
);
// All type checking constraints were added, try to fallback unsolved variables.
self.select_obligations_where_possible(false, |_| {});
let mut fallback_has_occurred = false;
debug!(
"type-inference-fallback post selection obligations: {:#?}",
self.fulfillment_cx.borrow_mut().pending_obligations()
);
// 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
// cause us to unify inference variables for opaque types,
// since we may have unified some other type variables
// during the first phase of fallback.
// This means that we only replace inference variables with their underlying
// opaque types as a last resort.
// We now see if we can make progress. This might cause us to
// unify inference variables for opaque types, since we may
// have unified some other type variables during the first
// phase of fallback. This means that we only replace
// inference variables with their underlying opaque types as a
// last resort.
//
// In code like this:
//
@ -62,36 +85,44 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
//
// - Unconstrained floats are replaced with with `f64`.
//
// - Non-numerics get replaced with `!` when `#![feature(never_type_fallback)]`
// is enabled. Otherwise, they are replaced with `()`.
// - Non-numerics may get replaced with `()` or `!`, depending on
// 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
// solely on how it was created, not what other type variables
// it may have been unified with since then.
// is an unsolved variable, and we determine its fallback
// based solely on how it was created, not what other type
// variables it may have been unified with since then.
//
// The reason this matters is that other attempts at fallback may
// (in principle) conflict with this fallback, and we wish to generate
// a type error in that case. (However, this actually isn't true right now,
// because we're only using the builtin fallback rules. This would be
// true if we were using user-supplied fallbacks. But it's still useful
// to write the code to detect bugs.)
// The reason this matters is that other attempts at fallback
// may (in principle) conflict with this fallback, and we wish
// to generate a type error in that case. (However, this
// actually isn't true right now, because we're only using the
// builtin fallback rules. This would be true if we were using
// 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
// with an integer type variable `?I` that ultimately never gets
// resolved to a special integral type, `?T` is not considered unsolved,
// but `?I` is. The same is true for float variables.)
// (Note though that if we have a general type variable `?T`
// that is then unified with an integer type variable `?I`
// that ultimately never gets resolved to a special integral
// 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) {
Diverging::Diverges => self.tcx.mk_diverging_default(),
Diverging::NotDiverging => return false,
_ => match diverging_fallback.get(&ty) {
Some(&fallback_ty) => fallback_ty,
None => return false,
},
};
debug!("fallback_if_possible(ty={:?}): defaulting to `{:?}`", ty, fallback);
@ -105,11 +136,10 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
true
}
/// Second round of fallback: Unconstrained type variables
/// created from the instantiation of an opaque
/// type fall back to the opaque type itself. This is a
/// somewhat incomplete attempt to manage "identity passthrough"
/// for `impl Trait` types.
/// Second round of fallback: Unconstrained type variables created
/// from the instantiation of an opaque type fall back to the
/// opaque type itself. This is a somewhat incomplete attempt to
/// manage "identity passthrough" for `impl Trait` types.
///
/// For example, in this code:
///
@ -158,4 +188,274 @@ 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);
let relationships = self.fulfillment_cx.borrow_mut().relationships().clone();
// 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 = unsolved_variables.iter().filter_map(|ty| ty.ty_vid());
// Compute the diverging root vids D -- that is, the root vid of
// those type variables that (a) are the target of a coercion from
// a `!` type and (b) have not yet been solved.
//
// These variables are the ones that are targets for fallback to
// either `!` or `()`.
let diverging_roots: FxHashSet<ty::TyVid> = self
.diverging_type_vars
.borrow()
.iter()
.map(|&ty| self.infcx.shallow_resolve(ty))
.filter_map(|ty| ty.ty_vid())
.map(|vid| self.infcx.root_var(vid))
.collect();
debug!(
"calculate_diverging_fallback: diverging_type_vars={:?}",
self.diverging_type_vars.borrow()
);
debug!("calculate_diverging_fallback: diverging_roots={:?}", diverging_roots);
// 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 = DepthFirstSearch::new(&coercion_graph);
let mut diverging_vids = vec![];
let mut non_diverging_vids = vec![];
for unsolved_vid in unsolved_vids {
let root_vid = self.infcx.root_var(unsolved_vid);
debug!(
"calculate_diverging_fallback: unsolved_vid={:?} root_vid={:?} diverges={:?}",
unsolved_vid,
root_vid,
diverging_roots.contains(&root_vid),
);
if diverging_roots.contains(&root_vid) {
diverging_vids.push(unsolved_vid);
roots_reachable_from_diverging.push_start_node(root_vid);
debug!(
"calculate_diverging_fallback: root_vid={:?} reaches {:?}",
root_vid,
coercion_graph.depth_first_search(root_vid).collect::<Vec<_>>()
);
// drain the iterator to visit all nodes reachable from this node
roots_reachable_from_diverging.complete_search();
} else {
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 = DepthFirstSearch::new(&coercion_graph);
for &non_diverging_vid in &non_diverging_vids {
let root_vid = self.infcx.root_var(non_diverging_vid);
if roots_reachable_from_diverging.visited(root_vid) {
continue;
}
roots_reachable_from_non_diverging.push_start_node(root_vid);
roots_reachable_from_non_diverging.complete_search();
}
debug!(
"calculate_diverging_fallback: roots_reachable_from_non_diverging={:?}",
roots_reachable_from_non_diverging,
);
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
// `!`.
let mut diverging_fallback = FxHashMap::default();
diverging_fallback.reserve(diverging_vids.len());
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.visited(n));
let mut relationship = ty::FoundRelationships { self_in_trait: false, output: false };
for (vid, rel) in relationships.iter() {
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 {
// This case falls back to () to ensure that the code pattern in
// src/test/ui/never_type/fallback-closure-ret.rs continues to
// compile when never_type_fallback is enabled.
//
// This rule is not readily explainable from first principles,
// but is rather intended as a patchwork fix to ensure code
// which compiles before the stabilization of never type
// fallback continues to work.
//
// Typically this pattern is encountered in a function taking a
// closure as a parameter, where the return type of that closure
// (checked by `relationship.output`) is expected to implement
// some trait (checked by `relationship.self_in_trait`). This
// can come up in non-closure cases too, so we do not limit this
// rule to specifically `FnOnce`.
//
// When the closure's body is something like `panic!()`, the
// return type would normally be inferred to `!`. However, it
// needs to fall back to `()` in order to still compile, as the
// trait is specifically implemented for `()` but not `!`.
//
// For details on the requirements for these relationships to be
// set, see the relationship finding module in
// compiler/rustc_trait_selection/src/traits/relationships.rs.
debug!("fallback to () - found trait and projection: {:?}", diverging_vid);
diverging_fallback.insert(diverging_ty, self.tcx.types.unit);
} else if can_reach_non_diverging {
debug!("fallback to () - reached non-diverging: {:?}", diverging_vid);
diverging_fallback.insert(diverging_ty, self.tcx.types.unit);
} else {
debug!("fallback to ! - all diverging: {:?}", 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| {
// We consider both subtyping and coercion to imply 'flow' from
// some position in the code `a` to a different position `b`.
// This is then used to determine which variables interact with
// live code, and as such must fall back to `()` to preserve
// soundness.
//
// In practice currently the two ways that this happens is
// coercion and subtyping.
let (a, b) = if let ty::PredicateKind::Coerce(ty::CoercePredicate { a, b }) = atom {
(a, b)
} else if let ty::PredicateKind::Subtype(ty::SubtypePredicate {
a_is_expected: _,
a,
b,
}) = atom
{
(a, b)
} 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()?))
}
}

9
compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
View file

@ -286,6 +286,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return;
}
for a in &adj {
if let Adjust::NeverToAny = a.kind {
if a.target.is_ty_var() {
self.diverging_type_vars.borrow_mut().insert(a.target);
debug!("apply_adjustments: adding `{:?}` as diverging type var", a.target);
}
}
}
let autoborrow_mut = adj.iter().any(|adj| {
matches!(
adj,

7
compiler/rustc_typeck/src/check/inherited.rs
View file

@ -1,6 +1,7 @@
use super::callee::DeferredCallResolution;
use super::MaybeInProgressTables;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def_id::{DefIdMap, LocalDefId};
use rustc_hir::HirIdMap;
@ -56,6 +57,11 @@ pub struct Inherited<'a, 'tcx> {
pub(super) constness: hir::Constness,
pub(super) body_id: Option<hir::BodyId>,
/// Whenever we introduce an adjustment from `!` into a type variable,
/// we record that type variable here. This is later used to inform
/// fallback. See the `fallback` module for details.
pub(super) diverging_type_vars: RefCell<FxHashSet<Ty<'tcx>>>,
}
impl<'a, 'tcx> Deref for Inherited<'a, 'tcx> {
@ -121,6 +127,7 @@ impl Inherited<'a, 'tcx> {
deferred_call_resolutions: RefCell::new(Default::default()),
deferred_cast_checks: RefCell::new(Vec::new()),
deferred_generator_interiors: RefCell::new(Vec::new()),
diverging_type_vars: RefCell::new(Default::default()),
constness,
body_id,
}