diff --git a/compiler/rustc_infer/src/traits/engine.rs b/compiler/rustc_infer/src/traits/engine.rs index 42333dc29bc..a12f7dc759c 100644 --- a/compiler/rustc_infer/src/traits/engine.rs +++ b/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>; + + fn relationships(&mut self) -> &mut FxHashMap; } pub trait TraitEngineExt<'tcx> { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 777c6035be8..cc81ddbcc01 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2090,3 +2090,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, +} diff --git a/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs b/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs index 9c962d30ce0..ec62ee40068 100644 --- a/compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs +++ b/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>, + + relationships: FxHashMap, } 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); } @@ -146,4 +153,8 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> { fn pending_obligations(&self) -> Vec> { self.obligations.iter().cloned().collect() } + + fn relationships(&mut self) -> &mut FxHashMap { + &mut self.relationships + } } diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs index b376f429292..61462f23886 100644 --- a/compiler/rustc_trait_selection/src/traits/fulfill.rs +++ b/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>, + + relationships: FxHashMap, + // 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> { self.predicates.map_pending_obligations(|o| o.obligation.clone()) } + + fn relationships(&mut self) -> &mut FxHashMap { + &mut self.relationships + } } struct FulfillProcessor<'a, 'b, 'tcx> { diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index ef208c44471..df2422048b9 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/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; diff --git a/compiler/rustc_trait_selection/src/traits/relationships.rs b/compiler/rustc_trait_selection/src/traits/relationships.rs new file mode 100644 index 00000000000..7751dd84f4c --- /dev/null +++ b/compiler/rustc_trait_selection/src/traits/relationships.rs @@ -0,0 +1,69 @@ +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}; + +pub(crate) fn update<'tcx, T>( + engine: &mut T, + infcx: &InferCtxt<'_, 'tcx>, + obligation: &PredicateObligation<'tcx>, +) where + T: TraitEngine<'tcx>, +{ + // (*) binder skipped + if let ty::PredicateKind::Trait(predicate) = obligation.predicate.kind().skip_binder() { + if let Some(ty) = + infcx.shallow_resolve(predicate.self_ty()).ty_vid().map(|t| infcx.root_var(t)) + { + if infcx + .tcx + .lang_items() + .sized_trait() + .map_or(false, |st| st != predicate.trait_ref.def_id) + { + 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(|_| { + // (*) binder moved here + 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() { + engine.relationships().entry(ty).or_default().self_in_trait = true; + } + } + } + } + } + + 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; + } + } +} diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index 508cea2845a..296e45337ed 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -11,9 +11,19 @@ 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, |_| {}); + 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() { @@ -251,6 +261,8 @@ impl<'tcx> FnCtxt<'_, 'tcx> { ) -> FxHashMap, 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(); @@ -334,21 +346,63 @@ impl<'tcx> FnCtxt<'_, 'tcx> { 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)); - if can_reach_non_diverging { - debug!("fallback to (): {:?}", diverging_vid); + + 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 !: {:?}", diverging_vid); + debug!("fallback to ! - all diverging: {:?}", diverging_vid); diverging_fallback.insert(diverging_ty, self.tcx.mk_diverging_default()); } } @@ -369,10 +423,23 @@ impl<'tcx> FnCtxt<'_, 'tcx> { 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)) + // 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; }; diff --git a/src/test/ui/never_type/fallback-closure-ret.rs b/src/test/ui/never_type/fallback-closure-ret.rs new file mode 100644 index 00000000000..5c8ce48cbb0 --- /dev/null +++ b/src/test/ui/never_type/fallback-closure-ret.rs @@ -0,0 +1,23 @@ +// This test verifies that never type fallback preserves the following code in a +// compiling state. This pattern is fairly common in the wild, notably seen in +// wasmtime v0.16. Typically this is some closure wrapper that expects a +// collection of 'known' signatures, and -> ! is not included in that set. +// +// This test is specifically targeted by the unit type fallback when +// encountering a set of obligations like `?T: Foo` and `Trait::Projection = +// ?T`. In the code below, these are `R: Bar` and `Fn::Output = R`. +// +// revisions: nofallback fallback +// check-pass + +#![cfg_attr(fallback, feature(never_type_fallback))] + +trait Bar { } +impl Bar for () { } +impl Bar for u32 { } + +fn foo(_: impl Fn() -> R) {} + +fn main() { + foo(|| panic!()); +} diff --git a/src/test/ui/never_type/fallback-closure-wrap.rs b/src/test/ui/never_type/fallback-closure-wrap.rs new file mode 100644 index 00000000000..af0577ac060 --- /dev/null +++ b/src/test/ui/never_type/fallback-closure-wrap.rs @@ -0,0 +1,30 @@ +// This is a minified example from Crater breakage observed when attempting to +// stabilize never type, nstoddard/webgl-gui @ 22f0169f. +// +// This particular test case currently fails as the inference to `()` rather +// than `!` happens as a result of an `as` cast, which is not currently tracked. +// Crater did not find many cases of this occuring, but it is included for +// awareness. +// +// revisions: nofallback fallback +//[nofallback] check-pass +//[fallback] check-fail + +#![cfg_attr(fallback, feature(never_type_fallback))] + +use std::marker::PhantomData; + +fn main() { + let error = Closure::wrap(Box::new(move || { + //[fallback]~^ ERROR type mismatch resolving + panic!("Can't connect to server."); + }) as Box); +} + +struct Closure(PhantomData); + +impl Closure { + fn wrap(data: Box) -> Closure { + todo!() + } +}