From 1cbde8566ef2ba9bd833ed22e50fc639cecc2fe2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 19 Nov 2020 18:24:03 -0500 Subject: [PATCH 01/16] factor fallback code into its own module --- compiler/rustc_typeck/src/check/fallback.rs | 50 +++++++++++++++++++++ compiler/rustc_typeck/src/check/mod.rs | 46 +------------------ 2 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 compiler/rustc_typeck/src/check/fallback.rs diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs new file mode 100644 index 0000000000000..fd8119391c447 --- /dev/null +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -0,0 +1,50 @@ +use crate::check::FallbackMode; +use crate::check::FnCtxt; + +pub(super) fn type_inference_fallback<'tcx>(fcx: &FnCtxt<'_, 'tcx>) { + // All type checking constraints were added, try to fallback unsolved variables. + fcx.select_obligations_where_possible(false, |_| {}); + 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 &fcx.unsolved_variables() { + debug!("unsolved_variable = {:?}", ty); + fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::NoOpaque); + } + // 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: + // + // ```rust + // type MyType = impl Copy; + // fn produce() -> MyType { true } + // fn bad_produce() -> MyType { panic!() } + // ``` + // + // we want to unify the opaque inference variable in `bad_produce` + // with the diverging fallback for `panic!` (e.g. `()` or `!`). + // This will produce a nice error message about conflicting concrete + // types for `MyType`. + // + // If we had tried to fallback the opaque inference variable to `MyType`, + // we will generate a confusing type-check error that does not explicitly + // refer to opaque types. + fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); + + // We now run fallback again, but this time we allow it to replace + // unconstrained opaque type variables, in addition to performing + // other kinds of fallback. + for ty in &fcx.unsolved_variables() { + fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::All); + } + + // See if we can make any more progress. + fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); +} diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 1479eadf1b067..4160f96c76533 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -75,6 +75,7 @@ mod diverges; pub mod dropck; mod expectation; mod expr; +mod fallback; mod fn_ctxt; mod gather_locals; mod generator_interior; @@ -562,50 +563,7 @@ fn typeck_with_fallback<'tcx>( fcx }; - // All type checking constraints were added, try to fallback unsolved variables. - fcx.select_obligations_where_possible(false, |_| {}); - 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 &fcx.unsolved_variables() { - fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::NoOpaque); - } - // 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: - // - // ```rust - // type MyType = impl Copy; - // fn produce() -> MyType { true } - // fn bad_produce() -> MyType { panic!() } - // ``` - // - // we want to unify the opaque inference variable in `bad_produce` - // with the diverging fallback for `panic!` (e.g. `()` or `!`). - // This will produce a nice error message about conflicting concrete - // types for `MyType`. - // - // If we had tried to fallback the opaque inference variable to `MyType`, - // we will generate a confusing type-check error that does not explicitly - // refer to opaque types. - fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); - - // We now run fallback again, but this time we allow it to replace - // unconstrained opaque type variables, in addition to performing - // other kinds of fallback. - for ty in &fcx.unsolved_variables() { - fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::All); - } - - // See if we can make any more progress. - fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); + fallback::type_inference_fallback(&fcx); // Even though coercion casts provide type hints, we check casts after fallback for // backwards compatibility. This makes fallback a stronger type hint than a cast coercion. From 0005746de123f506651e0e80790c0c28d7e036f3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 19 Nov 2020 18:26:00 -0500 Subject: [PATCH 02/16] convert to a method on FnCtxt --- compiler/rustc_typeck/src/check/fallback.rs | 88 +++++++++++---------- compiler/rustc_typeck/src/check/mod.rs | 2 +- 2 files changed, 46 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index fd8119391c447..5d884f1f54691 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -1,50 +1,52 @@ use crate::check::FallbackMode; use crate::check::FnCtxt; -pub(super) fn type_inference_fallback<'tcx>(fcx: &FnCtxt<'_, 'tcx>) { - // All type checking constraints were added, try to fallback unsolved variables. - fcx.select_obligations_where_possible(false, |_| {}); - let mut fallback_has_occurred = false; +impl<'tcx> FnCtxt<'_, 'tcx> { + pub(super) fn type_inference_fallback(&self) { + // All type checking constraints were added, try to fallback unsolved variables. + self.select_obligations_where_possible(false, |_| {}); + 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 &fcx.unsolved_variables() { - debug!("unsolved_variable = {:?}", ty); - fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::NoOpaque); - } - // 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: - // - // ```rust - // type MyType = impl Copy; - // fn produce() -> MyType { true } - // fn bad_produce() -> MyType { panic!() } - // ``` - // - // we want to unify the opaque inference variable in `bad_produce` - // with the diverging fallback for `panic!` (e.g. `()` or `!`). - // This will produce a nice error message about conflicting concrete - // types for `MyType`. - // - // If we had tried to fallback the opaque inference variable to `MyType`, - // we will generate a confusing type-check error that does not explicitly - // refer to opaque types. - fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); + // 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() { + debug!("unsolved_variable = {:?}", ty); + fallback_has_occurred |= self.fallback_if_possible(ty, FallbackMode::NoOpaque); + } + // 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: + // + // ```rust + // type MyType = impl Copy; + // fn produce() -> MyType { true } + // fn bad_produce() -> MyType { panic!() } + // ``` + // + // we want to unify the opaque inference variable in `bad_produce` + // with the diverging fallback for `panic!` (e.g. `()` or `!`). + // This will produce a nice error message about conflicting concrete + // types for `MyType`. + // + // If we had tried to fallback the opaque inference variable to `MyType`, + // we will generate a confusing type-check error that does not explicitly + // refer to opaque types. + self.select_obligations_where_possible(fallback_has_occurred, |_| {}); - // We now run fallback again, but this time we allow it to replace - // unconstrained opaque type variables, in addition to performing - // other kinds of fallback. - for ty in &fcx.unsolved_variables() { - fallback_has_occurred |= fcx.fallback_if_possible(ty, FallbackMode::All); - } + // We now run fallback again, but this time we allow it to replace + // unconstrained opaque type variables, in addition to performing + // other kinds of fallback. + for ty in &self.unsolved_variables() { + fallback_has_occurred |= self.fallback_if_possible(ty, FallbackMode::All); + } - // See if we can make any more progress. - fcx.select_obligations_where_possible(fallback_has_occurred, |_| {}); + // See if we can make any more progress. + self.select_obligations_where_possible(fallback_has_occurred, |_| {}); + } } diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 4160f96c76533..7547b98a740ef 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -563,7 +563,7 @@ fn typeck_with_fallback<'tcx>( fcx }; - fallback::type_inference_fallback(&fcx); + fcx.type_inference_fallback(); // Even though coercion casts provide type hints, we check casts after fallback for // backwards compatibility. This makes fallback a stronger type hint than a cast coercion. From d8a881be04ba0a30843c32718dc734e9f75f3f2e Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 20 Nov 2020 05:40:33 -0500 Subject: [PATCH 03/16] introduce a `Diverging` enum instead of a bool --- compiler/rustc_infer/src/infer/combine.rs | 6 +- compiler/rustc_infer/src/infer/mod.rs | 20 ++-- .../rustc_infer/src/infer/nll_relate/mod.rs | 4 +- .../rustc_infer/src/infer/type_variable.rs | 12 ++- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 100 ++++++++++-------- 5 files changed, 80 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_infer/src/infer/combine.rs b/compiler/rustc_infer/src/infer/combine.rs index 6a1715ef81899..fb9cd4457f209 100644 --- a/compiler/rustc_infer/src/infer/combine.rs +++ b/compiler/rustc_infer/src/infer/combine.rs @@ -22,7 +22,6 @@ // is also useful to track which value is the "expected" value in // terms of error reporting. -use super::equate::Equate; use super::glb::Glb; use super::lub::Lub; use super::sub::Sub; @@ -30,6 +29,7 @@ use super::type_variable::TypeVariableValue; use super::unify_key::replace_if_possible; use super::unify_key::{ConstVarValue, ConstVariableValue}; use super::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; +use super::{equate::Equate, type_variable::Diverging}; use super::{InferCtxt, MiscVariable, TypeTrace}; use crate::traits::{Obligation, PredicateObligations}; @@ -639,7 +639,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { .inner .borrow_mut() .type_variables() - .new_var(self.for_universe, false, origin); + .new_var(self.for_universe, Diverging::NotDiverging, origin); let u = self.tcx().mk_ty_var(new_var_id); debug!("generalize: replacing original vid={:?} with new={:?}", vid, u); Ok(u) @@ -858,7 +858,7 @@ impl TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> { *self.infcx.inner.borrow_mut().type_variables().var_origin(vid); let new_var_id = self.infcx.inner.borrow_mut().type_variables().new_var( self.for_universe, - false, + Diverging::NotDiverging, origin, ); let u = self.tcx().mk_ty_var(new_var_id); diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 6affe0e5463df..13439a52e3878 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -44,7 +44,7 @@ use self::region_constraints::{GenericKind, RegionConstraintData, VarInfos, Veri use self::region_constraints::{ RegionConstraintCollector, RegionConstraintStorage, RegionSnapshot, }; -use self::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; +use self::type_variable::{Diverging, TypeVariableOrigin, TypeVariableOriginKind}; pub mod at; pub mod canonical; @@ -641,10 +641,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { t.fold_with(&mut self.freshener()) } - pub fn type_var_diverges(&'a self, ty: Ty<'_>) -> bool { + 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), - _ => false, + _ => Diverging::NotDiverging, } } @@ -977,12 +977,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }) } - pub fn next_ty_var_id(&self, diverging: bool, origin: TypeVariableOrigin) -> TyVid { + pub fn next_ty_var_id(&self, diverging: Diverging, origin: TypeVariableOrigin) -> TyVid { self.inner.borrow_mut().type_variables().new_var(self.universe(), diverging, origin) } pub fn next_ty_var(&self, origin: TypeVariableOrigin) -> Ty<'tcx> { - self.tcx.mk_ty_var(self.next_ty_var_id(false, origin)) + self.tcx.mk_ty_var(self.next_ty_var_id(Diverging::NotDiverging, origin)) } pub fn next_ty_var_in_universe( @@ -990,12 +990,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { origin: TypeVariableOrigin, universe: ty::UniverseIndex, ) -> Ty<'tcx> { - let vid = self.inner.borrow_mut().type_variables().new_var(universe, false, origin); + let vid = self.inner.borrow_mut().type_variables().new_var( + universe, + Diverging::NotDiverging, + origin, + ); self.tcx.mk_ty_var(vid) } pub fn next_diverging_ty_var(&self, origin: TypeVariableOrigin) -> Ty<'tcx> { - self.tcx.mk_ty_var(self.next_ty_var_id(true, origin)) + self.tcx.mk_ty_var(self.next_ty_var_id(Diverging::Diverges, origin)) } pub fn next_const_var( @@ -1109,7 +1113,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // as the substitutions for the default, `(T, U)`. let ty_var_id = self.inner.borrow_mut().type_variables().new_var( self.universe(), - false, + Diverging::NotDiverging, TypeVariableOrigin { kind: TypeVariableOriginKind::TypeParameterDefinition( param.name, diff --git a/compiler/rustc_infer/src/infer/nll_relate/mod.rs b/compiler/rustc_infer/src/infer/nll_relate/mod.rs index 0b2847658f71e..18fd6f8c42706 100644 --- a/compiler/rustc_infer/src/infer/nll_relate/mod.rs +++ b/compiler/rustc_infer/src/infer/nll_relate/mod.rs @@ -22,6 +22,7 @@ //! constituents) use crate::infer::combine::ConstEquateRelation; +use crate::infer::type_variable::Diverging; use crate::infer::InferCtxt; use crate::infer::{ConstVarValue, ConstVariableValue}; use rustc_data_structures::fx::FxHashMap; @@ -898,7 +899,8 @@ where // Replacing with a new variable in the universe `self.universe`, // it will be unified later with the original type variable in // the universe `_universe`. - let new_var_id = variables.new_var(self.universe, false, origin); + let new_var_id = + variables.new_var(self.universe, Diverging::NotDiverging, origin); let u = self.tcx().mk_ty_var(new_var_id); debug!("generalize: replacing original vid={:?} with new={:?}", vid, u); diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index 35b97fff3da1f..6cf5bc8022e69 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -119,7 +119,13 @@ pub enum TypeVariableOriginKind { pub(crate) struct TypeVariableData { origin: TypeVariableOrigin, - diverging: bool, + diverging: Diverging, +} + +#[derive(Copy, Clone, Debug)] +pub enum Diverging { + NotDiverging, + Diverges, } #[derive(Copy, Clone, Debug)] @@ -175,7 +181,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { /// /// Note that this function does not return care whether /// `vid` has been unified with something else or not. - pub fn var_diverges(&self, vid: ty::TyVid) -> bool { + pub fn var_diverges(&self, vid: ty::TyVid) -> Diverging { self.storage.values.get(vid.index as usize).diverging } @@ -240,7 +246,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { pub fn new_var( &mut self, universe: ty::UniverseIndex, - diverging: bool, + diverging: Diverging, origin: TypeVariableOrigin, ) -> ty::TyVid { let eq_key = self.eq_relations().new_key(TypeVariableValue::Unknown { universe }); diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index e1a2f593b8d9b..7380585e941a7 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -14,8 +14,11 @@ use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_hir::{ExprKind, GenericArg, Node, QPath}; -use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryResponse}; use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282; +use rustc_infer::infer::{ + canonical::{Canonical, OriginalQueryValues, QueryResponse}, + type_variable::Diverging, +}; use rustc_infer::infer::{InferOk, InferResult}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::fold::TypeFoldable; @@ -630,56 +633,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ if self.is_tainted_by_errors() => self.tcx().ty_error(), UnconstrainedInt => self.tcx.types.i32, UnconstrainedFloat => self.tcx.types.f64, - Neither if self.type_var_diverges(ty) => self.tcx.mk_diverging_default(), - Neither => { - // This type variable was created from the instantiation of an opaque - // type. The fact that we're attempting to perform fallback for it - // means that the function neither constrained it to a concrete - // type, nor to the opaque type itself. - // - // For example, in this code: - // - //``` - // type MyType = impl Copy; - // fn defining_use() -> MyType { true } - // fn other_use() -> MyType { defining_use() } - // ``` - // - // `defining_use` will constrain the instantiated inference - // variable to `bool`, while `other_use` will constrain - // the instantiated inference variable to `MyType`. - // - // When we process opaque types during writeback, we - // will handle cases like `other_use`, and not count - // them as defining usages - // - // However, we also need to handle cases like this: - // - // ```rust - // pub type Foo = impl Copy; - // fn produce() -> Option { - // None - // } - // ``` - // - // In the above snippet, the inference variable created by - // instantiating `Option` will be completely unconstrained. - // We treat this as a non-defining use by making the inference - // variable fall back to the opaque type itself. - if let FallbackMode::All = mode { - if let Some(opaque_ty) = self.opaque_types_vars.borrow().get(ty) { - debug!( - "fallback_if_possible: falling back opaque type var {:?} to {:?}", - ty, opaque_ty - ); - *opaque_ty + Neither => match self.type_var_diverges(ty) { + Diverging::Diverges => self.tcx.mk_diverging_default(), + + Diverging::NotDiverging => { + // This type variable was created from the instantiation of an opaque + // type. The fact that we're attempting to perform fallback for it + // means that the function neither constrained it to a concrete + // type, nor to the opaque type itself. + // + // For example, in this code: + // + //``` + // type MyType = impl Copy; + // fn defining_use() -> MyType { true } + // fn other_use() -> MyType { defining_use() } + // ``` + // + // `defining_use` will constrain the instantiated inference + // variable to `bool`, while `other_use` will constrain + // the instantiated inference variable to `MyType`. + // + // When we process opaque types during writeback, we + // will handle cases like `other_use`, and not count + // them as defining usages + // + // However, we also need to handle cases like this: + // + // ```rust + // pub type Foo = impl Copy; + // fn produce() -> Option { + // None + // } + // ``` + // + // In the above snippet, the inference variable created by + // instantiating `Option` will be completely unconstrained. + // We treat this as a non-defining use by making the inference + // variable fall back to the opaque type itself. + if let FallbackMode::All = mode { + if let Some(opaque_ty) = self.opaque_types_vars.borrow().get(ty) { + debug!( + "fallback_if_possible: falling back opaque type var {:?} to {:?}", + ty, opaque_ty + ); + *opaque_ty + } else { + return false; + } } else { return false; } - } else { - return false; } - } + }, }; debug!("fallback_if_possible: defaulting `{:?}` to `{:?}`", ty, fallback); self.demand_eqtype(rustc_span::DUMMY_SP, ty, fallback); From 00786cf01ad36bd5de0c32e212c93ce3298617ad Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 20 Nov 2020 05:52:27 -0500 Subject: [PATCH 04/16] shallow resolve target type in coercion We used to avoid doing this because we didn't want to make coercion depend on the state of inference. For better or worse, we have moved away from this position over time. Therefore, I am going to go ahead and resolve the `b` target type early on so that it is done uniformly. (The older technique for managing this was always something of a hack regardless; if we really wanted to avoid integrating coercion and inference we needed to be more disciplined about it.) --- compiler/rustc_typeck/src/check/coercion.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 0f5f0ab026087..c05619eed0838 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -146,6 +146,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { fn coerce(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> { let a = self.shallow_resolve(a); + let b = self.shallow_resolve(b); debug!("Coerce.tys({:?} => {:?})", a, b); // Just ignore error types. @@ -161,8 +162,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { // let _: Option = Some({ return; }); // // here, we would coerce from `!` to `?T`. - let b = self.shallow_resolve(b); - return if self.shallow_resolve(b).is_ty_var() { + 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 { @@ -195,9 +195,6 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { debug!("coerce: unsize failed"); // Examine the supertype and consider auto-borrowing. - // - // Note: does not attempt to resolve type variables we encounter. - // See above for details. match *b.kind() { ty::RawPtr(mt_b) => { return self.coerce_unsafe_ptr(a, b, mt_b.mutbl); From b3f8d74e27fa33ffbf97f174d009eb33e02a3e8b Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 21 Nov 2020 05:45:13 -0500 Subject: [PATCH 05/16] cleanup: extract a helper for coercion from inference variables --- compiler/rustc_typeck/src/check/coercion.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index c05619eed0838..d6cd2197d9a8f 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -145,6 +145,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { } fn coerce(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> { + // First, remove any resolved type variables (at the top level, at least): let a = self.shallow_resolve(a); let b = self.shallow_resolve(b); debug!("Coerce.tys({:?} => {:?})", a, b); @@ -154,6 +155,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { return success(vec![], self.fcx.tcx.ty_error(), vec![]); } + // 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 @@ -175,6 +177,13 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { }; } + // Coercing *from* an unresolved inference variable means that + // we have no information about the source type. This will always + // ultimately fall back to some form of subtyping. + if a.is_ty_var() { + return self.coerce_from_inference_variable(a, b); + } + // Consider coercing the subtype to a DST // // NOTE: this is wrapped in a `commit_if_ok` because it creates @@ -232,6 +241,16 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { } } + /// Coercing *from* an inference variable. In this case, we have no information + /// about the source type, so we can't really do a true coercion and we always + /// fall back to subtyping (`unify_and`). + fn coerce_from_inference_variable(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> { + assert!(a.is_ty_var() && self.infcx.shallow_resolve(a) == a); + assert!(self.infcx.shallow_resolve(b) == b); + + self.unify_and(a, b, identity) + } + /// Reborrows `&mut A` to `&mut B` and `&(mut) A` to `&B`. /// To match `A` with `B`, autoderef will be performed, /// calling `deref`/`deref_mut` where necessary. From 448eb370a52376346d50bbbfb8abd80f8789c4f8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 21 Nov 2020 07:06:16 -0500 Subject: [PATCH 06/16] introduce a Coerce predicate --- compiler/rustc_infer/src/infer/mod.rs | 29 ++++ .../rustc_infer/src/infer/outlives/mod.rs | 1 + compiler/rustc_infer/src/traits/util.rs | 4 + compiler/rustc_lint/src/builtin.rs | 1 + compiler/rustc_middle/src/ty/flags.rs | 4 + compiler/rustc_middle/src/ty/mod.rs | 28 ++++ compiler/rustc_middle/src/ty/print/pretty.rs | 5 + .../rustc_middle/src/ty/structural_impls.rs | 9 + .../src/transform/check_consts/validation.rs | 4 +- .../rustc_trait_selection/src/opaque_types.rs | 1 + .../src/traits/error_reporting/mod.rs | 7 + .../src/traits/fulfill.rs | 26 +++ .../src/traits/object_safety.rs | 2 + .../src/traits/select/mod.rs | 16 ++ .../rustc_trait_selection/src/traits/wf.rs | 4 + compiler/rustc_traits/src/chalk/lowering.rs | 4 + .../src/implied_outlives_bounds.rs | 1 + .../src/normalize_erasing_regions.rs | 1 + .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 1 + .../rustc_typeck/src/check/method/probe.rs | 1 + .../src/impl_wf_check/min_specialization.rs | 1 + .../rustc_typeck/src/outlives/explicit.rs | 1 + src/librustdoc/clean/mod.rs | 1 + .../src/utils/qualify_min_const_fn.rs | 155 +++++++++++------- 24 files changed, 247 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 13439a52e3878..5a978a819334f 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -926,6 +926,35 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ); } + /// Processes a `Coerce` predicate from the fulfillment context. + /// This is NOT the preferred way to handle coercion, which is to + /// invoke `FnCtxt::coerce` or a similar method (see `coercion.rs`). + /// + /// This method here is actually a fallback that winds up being + /// invoked when `FnCtxt::coerce` encounters unresolved type variables + /// and records a coercion predicate. Presently, this method is equivalent + /// to `subtype_predicate` -- that is, "coercing" `a` to `b` winds up + /// actually requiring `a <: b`. This is of course a valid coercion, + /// but it's not as flexible as `FnCtxt::coerce` would be. + /// + /// (We may refactor this in the future, but there are a number of + /// practical obstacles. Among other things, `FnCtxt::coerce` presently + /// records adjustments that are required on the HIR in order to perform + /// the coercion, and we don't currently have a way to manage that.) + pub fn coerce_predicate( + &self, + cause: &ObligationCause<'tcx>, + param_env: ty::ParamEnv<'tcx>, + predicate: ty::PolyCoercePredicate<'tcx>, + ) -> Option> { + let subtype_predicate = predicate.map_bound(|p| ty::SubtypePredicate { + a_is_expected: false, // when coercing from `a` to `b`, `b` is expected + a: p.a, + b: p.b, + }); + self.subtype_predicate(cause, param_env, subtype_predicate) + } + pub fn subtype_predicate( &self, cause: &ObligationCause<'tcx>, diff --git a/compiler/rustc_infer/src/infer/outlives/mod.rs b/compiler/rustc_infer/src/infer/outlives/mod.rs index de98cccf25689..d6af58c2b7c3e 100644 --- a/compiler/rustc_infer/src/infer/outlives/mod.rs +++ b/compiler/rustc_infer/src/infer/outlives/mod.rs @@ -21,6 +21,7 @@ pub fn explicit_outlives_bounds<'tcx>( ty::PredicateAtom::Projection(..) | ty::PredicateAtom::Trait(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::WellFormed(..) | ty::PredicateAtom::ObjectSafe(..) | ty::PredicateAtom::ClosureKind(..) diff --git a/compiler/rustc_infer/src/traits/util.rs b/compiler/rustc_infer/src/traits/util.rs index b0b0e4372b8cd..751bf3a8e8cb7 100644 --- a/compiler/rustc_infer/src/traits/util.rs +++ b/compiler/rustc_infer/src/traits/util.rs @@ -162,6 +162,10 @@ impl Elaborator<'tcx> { // Currently, we do not "elaborate" predicates like `X <: Y`, // though conceivably we might. } + ty::PredicateAtom::Coerce(..) => { + // Currently, we do not "elaborate" predicates like `X -> Y`, + // though conceivably we might. + } ty::PredicateAtom::Projection(..) => { // Nothing to elaborate in a projection predicate. } diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index c65cf65b1c777..cc1872473e693 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -1568,6 +1568,7 @@ impl<'tcx> LateLintPass<'tcx> for TrivialConstraints { ObjectSafe(..) | ClosureKind(..) | Subtype(..) | + Coerce(..) | ConstEvaluatable(..) | ConstEquate(..) | TypeWellFormedFromEnv(..) => continue, diff --git a/compiler/rustc_middle/src/ty/flags.rs b/compiler/rustc_middle/src/ty/flags.rs index 8b97a87f214b8..30d342d65cf0a 100644 --- a/compiler/rustc_middle/src/ty/flags.rs +++ b/compiler/rustc_middle/src/ty/flags.rs @@ -236,6 +236,10 @@ impl FlagComputation { self.add_ty(a); self.add_ty(b); } + ty::PredicateAtom::Coerce(ty::CoercePredicate { a, b }) => { + self.add_ty(a); + self.add_ty(b); + } ty::PredicateAtom::Projection(ty::ProjectionPredicate { projection_ty, ty }) => { self.add_projection_ty(projection_ty); self.add_ty(ty); diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 6a67935cd98fe..b3366d0c69092 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1208,8 +1208,22 @@ pub enum PredicateAtom<'tcx> { ClosureKind(DefId, SubstsRef<'tcx>, ClosureKind), /// `T1 <: T2` + /// + /// This obligation is created most often when we have two + /// unresolved type variables and hence don't have enough + /// information to process the subtyping obligation yet. Subtype(SubtypePredicate<'tcx>), + /// `T1` coerced to `T2` + /// + /// Like a subtyping obligation, thi sis created most often + /// when we have two unresolved type variables and hence + /// don't have enough information to process the coercion + /// obligation yet. At the moment, we actually process coercions + /// very much like subtyping and don't handle the full coercion + /// logic. + Coerce(CoercePredicate<'tcx>), + /// Constant initializer must evaluate successfully. ConstEvaluatable(ty::WithOptConstParam, SubstsRef<'tcx>), @@ -1362,6 +1376,9 @@ pub type TypeOutlivesPredicate<'tcx> = OutlivesPredicate, ty::Region<'t pub type PolyRegionOutlivesPredicate<'tcx> = ty::Binder>; pub type PolyTypeOutlivesPredicate<'tcx> = ty::Binder>; +/// Encodes that `a` must be a subtype of `b`. The `a_is_expected` flag indicates +/// whether the `a` type is the type that we should label as "expected" when +/// presenting user diagnostics. #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, TyEncodable, TyDecodable)] #[derive(HashStable, TypeFoldable)] pub struct SubtypePredicate<'tcx> { @@ -1371,6 +1388,15 @@ pub struct SubtypePredicate<'tcx> { } pub type PolySubtypePredicate<'tcx> = ty::Binder>; +/// Encodes that we have to coerce *from* the `a` type to the `b` type. +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, TyEncodable, TyDecodable)] +#[derive(HashStable, TypeFoldable)] +pub struct CoercePredicate<'tcx> { + pub a: Ty<'tcx>, + pub b: Ty<'tcx>, +} +pub type PolyCoercePredicate<'tcx> = ty::Binder>; + /// This kind of predicate has no *direct* correspondent in the /// syntax, but it roughly corresponds to the syntactic forms: /// @@ -1508,6 +1534,7 @@ impl<'tcx> Predicate<'tcx> { PredicateAtom::Trait(t, _) => Some(ty::Binder::bind(t.trait_ref)), PredicateAtom::Projection(..) | PredicateAtom::Subtype(..) + | PredicateAtom::Coerce(..) | PredicateAtom::RegionOutlives(..) | PredicateAtom::WellFormed(..) | PredicateAtom::ObjectSafe(..) @@ -1525,6 +1552,7 @@ impl<'tcx> Predicate<'tcx> { PredicateAtom::Trait(..) | PredicateAtom::Projection(..) | PredicateAtom::Subtype(..) + | PredicateAtom::Coerce(..) | PredicateAtom::RegionOutlives(..) | PredicateAtom::WellFormed(..) | PredicateAtom::ObjectSafe(..) diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 38f8e779f6a92..5587a25f16d21 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -2003,6 +2003,10 @@ define_print_and_forward_display! { p!(print(self.a), " <: ", print(self.b)) } + ty::CoercePredicate<'tcx> { + p!(print(self.a), " -> ", print(self.b)) + } + ty::TraitPredicate<'tcx> { p!(print(self.trait_ref.self_ty()), ": ", print(self.trait_ref.print_only_trait_path())) @@ -2040,6 +2044,7 @@ define_print_and_forward_display! { p!(print(data)) } ty::PredicateAtom::Subtype(predicate) => p!(print(predicate)), + ty::PredicateAtom::Coerce(predicate) => p!(print(predicate)), ty::PredicateAtom::RegionOutlives(predicate) => p!(print(predicate)), ty::PredicateAtom::TypeOutlives(predicate) => p!(print(predicate)), ty::PredicateAtom::Projection(predicate) => p!(print(predicate)), diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 94e69a93a6b18..713191264c7d5 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -247,6 +247,7 @@ impl fmt::Debug for ty::PredicateAtom<'tcx> { a.fmt(f) } ty::PredicateAtom::Subtype(ref pair) => pair.fmt(f), + ty::PredicateAtom::Coerce(ref pair) => pair.fmt(f), ty::PredicateAtom::RegionOutlives(ref pair) => pair.fmt(f), ty::PredicateAtom::TypeOutlives(ref pair) => pair.fmt(f), ty::PredicateAtom::Projection(ref pair) => pair.fmt(f), @@ -447,6 +448,13 @@ impl<'a, 'tcx> Lift<'tcx> for ty::SubtypePredicate<'a> { } } +impl<'a, 'tcx> Lift<'tcx> for ty::CoercePredicate<'a> { + type Lifted = ty::CoercePredicate<'tcx>; + fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option> { + tcx.lift((self.a, self.b)).map(|(a, b)| ty::CoercePredicate { a, b }) + } +} + impl<'tcx, A: Copy + Lift<'tcx>, B: Copy + Lift<'tcx>> Lift<'tcx> for ty::OutlivesPredicate { type Lifted = ty::OutlivesPredicate; fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option { @@ -499,6 +507,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::PredicateAtom<'a> { tcx.lift(data).map(|data| ty::PredicateAtom::Trait(data, constness)) } ty::PredicateAtom::Subtype(data) => tcx.lift(data).map(ty::PredicateAtom::Subtype), + ty::PredicateAtom::Coerce(data) => tcx.lift(data).map(ty::PredicateAtom::Coerce), ty::PredicateAtom::RegionOutlives(data) => { tcx.lift(data).map(ty::PredicateAtom::RegionOutlives) } diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index e4893044a1599..9a2a1307f99c2 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -397,8 +397,8 @@ impl Validator<'mir, 'tcx> { ty::PredicateAtom::ClosureKind(..) => { bug!("closure kind predicate on function: {:#?}", predicate) } - ty::PredicateAtom::Subtype(_) => { - bug!("subtype predicate on function: {:#?}", predicate) + ty::PredicateAtom::Subtype(_) | ty::PredicateAtom::Coerce(_) => { + bug!("subtype/coerce predicate on function: {:#?}", predicate) } ty::PredicateAtom::Trait(pred, constness) => { if Some(pred.def_id()) == tcx.lang_items().sized_trait() { diff --git a/compiler/rustc_trait_selection/src/opaque_types.rs b/compiler/rustc_trait_selection/src/opaque_types.rs index ca547bf88b588..37112f6fd6b95 100644 --- a/compiler/rustc_trait_selection/src/opaque_types.rs +++ b/compiler/rustc_trait_selection/src/opaque_types.rs @@ -1265,6 +1265,7 @@ crate fn required_region_bounds( ty::PredicateAtom::Projection(..) | ty::PredicateAtom::Trait(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::WellFormed(..) | ty::PredicateAtom::ObjectSafe(..) | ty::PredicateAtom::ClosureKind(..) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index fe4127fd4d8b9..d4a3a22b0b9cf 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -532,6 +532,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { span_bug!(span, "subtype requirement gave wrong error: `{:?}`", predicate) } + ty::PredicateAtom::Coerce(predicate) => { + // Errors for Coerce predicates show up as + // `FulfillmentErrorCode::CodeSubtypeError`, + // not selection error. + span_bug!(span, "coerce requirement gave wrong error: `{:?}`", predicate) + } + ty::PredicateAtom::RegionOutlives(predicate) => { let predicate = bound_predicate.rebind(predicate); let predicate = self.resolve_vars_if_possible(predicate); diff --git a/compiler/rustc_trait_selection/src/traits/fulfill.rs b/compiler/rustc_trait_selection/src/traits/fulfill.rs index a04f816b0f8a0..b6b3d4d94b6d3 100644 --- a/compiler/rustc_trait_selection/src/traits/fulfill.rs +++ b/compiler/rustc_trait_selection/src/traits/fulfill.rs @@ -373,6 +373,7 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> { | ty::PredicateAtom::ObjectSafe(_) | ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::Subtype(_) + | ty::PredicateAtom::Coerce(_) | ty::PredicateAtom::ConstEvaluatable(..) | ty::PredicateAtom::ConstEquate(..) => { let pred = infcx.replace_bound_vars_with_placeholders(binder); @@ -487,6 +488,31 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> { } } + ty::PredicateAtom::Coerce(coerce) => { + match self.selcx.infcx().coerce_predicate( + &obligation.cause, + obligation.param_env, + Binder::dummy(coerce), + ) { + None => { + // None means that both are unresolved. + pending_obligation.stalled_on = vec![ + TyOrConstInferVar::maybe_from_ty(coerce.a).unwrap(), + TyOrConstInferVar::maybe_from_ty(coerce.b).unwrap(), + ]; + ProcessResult::Unchanged + } + Some(Ok(ok)) => ProcessResult::Changed(mk_pending(ok.obligations)), + Some(Err(err)) => { + let expected_found = ExpectedFound::new(false, coerce.a, coerce.b); + ProcessResult::Error(FulfillmentErrorCode::CodeSubtypeError( + expected_found, + err, + )) + } + } + } + ty::PredicateAtom::ConstEvaluatable(def_id, substs) => { match const_evaluatable::is_const_evaluatable( self.selcx.infcx(), diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index d912a00d6b702..93dc49ff4a362 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -306,6 +306,7 @@ fn predicate_references_self( | ty::PredicateAtom::RegionOutlives(..) | ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::ConstEvaluatable(..) | ty::PredicateAtom::ConstEquate(..) | ty::PredicateAtom::TypeWellFormedFromEnv(..) => None, @@ -334,6 +335,7 @@ fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool { } ty::PredicateAtom::Projection(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::RegionOutlives(..) | ty::PredicateAtom::WellFormed(..) | ty::PredicateAtom::ObjectSafe(..) diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 05ff9a6fb9ca6..9fff65ae197d3 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -474,6 +474,22 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } } + ty::PredicateAtom::Coerce(p) => { + let p = bound_predicate.rebind(p); + // Does this code ever run? + match self.infcx.coerce_predicate(&obligation.cause, obligation.param_env, p) { + Some(Ok(InferOk { mut obligations, .. })) => { + self.add_depth(obligations.iter_mut(), obligation.recursion_depth); + self.evaluate_predicates_recursively( + previous_stack, + obligations.into_iter(), + ) + } + Some(Err(_)) => Ok(EvaluatedToErr), + None => Ok(EvaluatedToAmbig), + } + } + ty::PredicateAtom::WellFormed(arg) => match wf::obligations( self.infcx, obligation.param_env, diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index e5a792f229d19..535f17696061d 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -127,6 +127,10 @@ pub fn predicate_obligations<'a, 'tcx>( wf.compute(a.into()); wf.compute(b.into()); } + ty::PredicateAtom::Coerce(ty::CoercePredicate { a, b }) => { + wf.compute(a.into()); + wf.compute(b.into()); + } ty::PredicateAtom::ConstEvaluatable(def, substs) => { let obligations = wf.nominal_obligations(def.did, substs); wf.out.extend(obligations); diff --git a/compiler/rustc_traits/src/chalk/lowering.rs b/compiler/rustc_traits/src/chalk/lowering.rs index 9afb980f84d27..f685bfe2416f1 100644 --- a/compiler/rustc_traits/src/chalk/lowering.rs +++ b/compiler/rustc_traits/src/chalk/lowering.rs @@ -114,6 +114,7 @@ impl<'tcx> LowerInto<'tcx, chalk_ir::InEnvironment bug!("unexpected predicate {}", predicate), }; @@ -201,6 +202,7 @@ impl<'tcx> LowerInto<'tcx, chalk_ir::GoalData>> for ty::Predi // some of these in terms of chalk operations. ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::ConstEvaluatable(..) | ty::PredicateAtom::ConstEquate(..) => { chalk_ir::GoalData::All(chalk_ir::Goals::empty(interner)) @@ -604,6 +606,7 @@ impl<'tcx> LowerInto<'tcx, Option { @@ -727,6 +730,7 @@ impl<'tcx> LowerInto<'tcx, Option { diff --git a/compiler/rustc_traits/src/implied_outlives_bounds.rs b/compiler/rustc_traits/src/implied_outlives_bounds.rs index 97017fbf2e56a..b9ec087025b06 100644 --- a/compiler/rustc_traits/src/implied_outlives_bounds.rs +++ b/compiler/rustc_traits/src/implied_outlives_bounds.rs @@ -99,6 +99,7 @@ fn compute_implied_outlives_bounds<'tcx>( &ty::PredicateKind::Atom(atom) => match atom { ty::PredicateAtom::Trait(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::Projection(..) | ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::ObjectSafe(..) diff --git a/compiler/rustc_traits/src/normalize_erasing_regions.rs b/compiler/rustc_traits/src/normalize_erasing_regions.rs index 750a0922be4d1..0600d1f820d53 100644 --- a/compiler/rustc_traits/src/normalize_erasing_regions.rs +++ b/compiler/rustc_traits/src/normalize_erasing_regions.rs @@ -48,6 +48,7 @@ fn not_outlives_predicate(p: &ty::Predicate<'tcx>) -> bool { | ty::PredicateAtom::ObjectSafe(..) | ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::ConstEvaluatable(..) | ty::PredicateAtom::ConstEquate(..) | ty::PredicateAtom::TypeWellFormedFromEnv(..) => true, diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index 7380585e941a7..c6c73e793be2e 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -778,6 +778,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Some((ty::Binder::bind(data).to_poly_trait_ref(), obligation)) } ty::PredicateAtom::Subtype(..) => None, + ty::PredicateAtom::Coerce(..) => None, ty::PredicateAtom::RegionOutlives(..) => None, ty::PredicateAtom::TypeOutlives(..) => None, ty::PredicateAtom::WellFormed(..) => None, diff --git a/compiler/rustc_typeck/src/check/method/probe.rs b/compiler/rustc_typeck/src/check/method/probe.rs index 478f8a16169ed..52e9f0d08cc47 100644 --- a/compiler/rustc_typeck/src/check/method/probe.rs +++ b/compiler/rustc_typeck/src/check/method/probe.rs @@ -807,6 +807,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { } } ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::Projection(..) | ty::PredicateAtom::RegionOutlives(..) | ty::PredicateAtom::WellFormed(..) diff --git a/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs b/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs index 5db9ff9524de0..d4615f5d35a03 100644 --- a/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs @@ -403,6 +403,7 @@ fn trait_predicate_kind<'tcx>( | ty::PredicateAtom::Projection(_) | ty::PredicateAtom::WellFormed(_) | ty::PredicateAtom::Subtype(_) + | ty::PredicateAtom::Coerce(_) | ty::PredicateAtom::ObjectSafe(_) | ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::ConstEvaluatable(..) diff --git a/compiler/rustc_typeck/src/outlives/explicit.rs b/compiler/rustc_typeck/src/outlives/explicit.rs index ae336ccca457d..fc8bd66ba78a2 100644 --- a/compiler/rustc_typeck/src/outlives/explicit.rs +++ b/compiler/rustc_typeck/src/outlives/explicit.rs @@ -56,6 +56,7 @@ impl<'tcx> ExplicitPredicatesMap<'tcx> { | ty::PredicateAtom::ObjectSafe(..) | ty::PredicateAtom::ClosureKind(..) | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::ConstEvaluatable(..) | ty::PredicateAtom::ConstEquate(..) | ty::PredicateAtom::TypeWellFormedFromEnv(..) => (), diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 9d84089eb405c..063831e86d500 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -499,6 +499,7 @@ impl<'a> Clean> for ty::Predicate<'a> { ty::PredicateAtom::Projection(pred) => Some(pred.clean(cx)), ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::Coerce(..) | ty::PredicateAtom::WellFormed(..) | ty::PredicateAtom::ObjectSafe(..) | ty::PredicateAtom::ClosureKind(..) diff --git a/src/tools/clippy/clippy_lints/src/utils/qualify_min_const_fn.rs b/src/tools/clippy/clippy_lints/src/utils/qualify_min_const_fn.rs index 7cb7d0a26b65e..449531380fff4 100644 --- a/src/tools/clippy/clippy_lints/src/utils/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_lints/src/utils/qualify_min_const_fn.rs @@ -1,8 +1,8 @@ use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::mir::{ - Body, CastKind, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, - TerminatorKind, + Body, CastKind, NullOp, Operand, Place, ProjectionElem, Rvalue, Statement, StatementKind, + Terminator, TerminatorKind, }; use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::{self, adjustment::PointerCast, Ty, TyCtxt}; @@ -27,9 +27,18 @@ pub fn is_min_const_fn(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>) -> McfResult { | ty::PredicateAtom::ConstEvaluatable(..) | ty::PredicateAtom::ConstEquate(..) | ty::PredicateAtom::TypeWellFormedFromEnv(..) => continue, - ty::PredicateAtom::ObjectSafe(_) => panic!("object safe predicate on function: {:#?}", predicate), - ty::PredicateAtom::ClosureKind(..) => panic!("closure kind predicate on function: {:#?}", predicate), - ty::PredicateAtom::Subtype(_) => panic!("subtype predicate on function: {:#?}", predicate), + ty::PredicateAtom::ObjectSafe(_) => { + panic!("object safe predicate on function: {:#?}", predicate) + } + ty::PredicateAtom::ClosureKind(..) => { + panic!("closure kind predicate on function: {:#?}", predicate) + } + ty::PredicateAtom::Subtype(_) => { + panic!("subtype predicate on function: {:#?}", predicate) + } + ty::PredicateAtom::Coerce(_) => { + panic!("coerce predicate on function: {:#?}", predicate) + } ty::PredicateAtom::Trait(pred, _) => { if Some(pred.def_id()) == tcx.lang_items().sized_trait() { continue; @@ -45,12 +54,12 @@ pub fn is_min_const_fn(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>) -> McfResult { on const fn parameters are unstable" .into(), )); - }, + } // other kinds of bounds are either tautologies // or cause errors in other passes _ => continue, } - }, + } } } match predicates.parent { @@ -91,22 +100,23 @@ fn check_ty(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, span: Span) -> McfResult { match ty.kind() { ty::Ref(_, _, hir::Mutability::Mut) => { return Err((span, "mutable references in const fn are unstable".into())); - }, + } ty::Opaque(..) => return Err((span, "`impl Trait` in const fn is unstable".into())), ty::FnPtr(..) => { return Err((span, "function pointers in const fn are unstable".into())); - }, + } ty::Dynamic(preds, _) => { for pred in preds.iter() { match pred.skip_binder() { - ty::ExistentialPredicate::AutoTrait(_) | ty::ExistentialPredicate::Projection(_) => { + ty::ExistentialPredicate::AutoTrait(_) + | ty::ExistentialPredicate::Projection(_) => { return Err(( span, "trait bounds other than `Sized` \ on const fn parameters are unstable" .into(), )); - }, + } ty::ExistentialPredicate::Trait(trait_ref) => { if Some(trait_ref.def_id) != tcx.lang_items().sized_trait() { return Err(( @@ -116,23 +126,34 @@ fn check_ty(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, span: Span) -> McfResult { .into(), )); } - }, + } } } - }, - _ => {}, + } + _ => {} } } Ok(()) } -fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rvalue<'tcx>, span: Span) -> McfResult { +fn check_rvalue( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + def_id: DefId, + rvalue: &Rvalue<'tcx>, + span: Span, +) -> McfResult { match rvalue { - Rvalue::ThreadLocalRef(_) => Err((span, "cannot access thread local storage in const fn".into())), - Rvalue::Repeat(operand, _) | Rvalue::Use(operand) => check_operand(tcx, operand, span, body), - Rvalue::Len(place) | Rvalue::Discriminant(place) | Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => { - check_place(tcx, *place, span, body) - }, + Rvalue::ThreadLocalRef(_) => { + Err((span, "cannot access thread local storage in const fn".into())) + } + Rvalue::Repeat(operand, _) | Rvalue::Use(operand) => { + check_operand(tcx, operand, span, body) + } + Rvalue::Len(place) + | Rvalue::Discriminant(place) + | Rvalue::Ref(_, _, place) + | Rvalue::AddressOf(_, place) => check_place(tcx, *place, span, body), Rvalue::Cast(CastKind::Misc, operand, cast_ty) => { use rustc_middle::ty::cast::CastTy; let cast_in = CastTy::from_ty(operand.ty(body, tcx)).expect("bad input type for cast"); @@ -140,16 +161,20 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv match (cast_in, cast_out) { (CastTy::Ptr(_) | CastTy::FnPtr, CastTy::Int(_)) => { Err((span, "casting pointers to ints is unstable in const fn".into())) - }, + } _ => check_operand(tcx, operand, span, body), } - }, - Rvalue::Cast(CastKind::Pointer(PointerCast::MutToConstPointer | PointerCast::ArrayToPointer), operand, _) => { - check_operand(tcx, operand, span, body) - }, + } + Rvalue::Cast( + CastKind::Pointer(PointerCast::MutToConstPointer | PointerCast::ArrayToPointer), + operand, + _, + ) => check_operand(tcx, operand, span, body), Rvalue::Cast( CastKind::Pointer( - PointerCast::UnsafeFnPointer | PointerCast::ClosureFnPointer(_) | PointerCast::ReifyFnPointer, + PointerCast::UnsafeFnPointer + | PointerCast::ClosureFnPointer(_) + | PointerCast::ReifyFnPointer, ), _, _, @@ -159,7 +184,10 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv deref_ty.ty } else { // We cannot allow this for now. - return Err((span, "unsizing casts are only allowed for references right now".into())); + return Err(( + span, + "unsizing casts are only allowed for references right now".into(), + )); }; let unsized_ty = tcx.struct_tail_erasing_lifetimes(pointee_ty, tcx.param_env(def_id)); if let ty::Slice(_) | ty::Str = unsized_ty.kind() { @@ -170,7 +198,7 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv // We just can't allow trait objects until we have figured out trait method calls. Err((span, "unsizing casts are not allowed in const fn".into())) } - }, + } // binops are fine on integers Rvalue::BinaryOp(_, lhs, rhs) | Rvalue::CheckedBinaryOp(_, lhs, rhs) => { check_operand(tcx, lhs, span, body)?; @@ -179,14 +207,13 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv if ty.is_integral() || ty.is_bool() || ty.is_char() { Ok(()) } else { - Err(( - span, - "only int, `bool` and `char` operations are stable in const fn".into(), - )) + Err((span, "only int, `bool` and `char` operations are stable in const fn".into())) } - }, + } Rvalue::NullaryOp(NullOp::SizeOf, _) => Ok(()), - Rvalue::NullaryOp(NullOp::Box, _) => Err((span, "heap allocations are not allowed in const fn".into())), + Rvalue::NullaryOp(NullOp::Box, _) => { + Err((span, "heap allocations are not allowed in const fn".into())) + } Rvalue::UnaryOp(_, operand) => { let ty = operand.ty(body, tcx); if ty.is_integral() || ty.is_bool() { @@ -194,17 +221,22 @@ fn check_rvalue(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, rvalue: &Rv } else { Err((span, "only int and `bool` operations are stable in const fn".into())) } - }, + } Rvalue::Aggregate(_, operands) => { for operand in operands { check_operand(tcx, operand, span, body)?; } Ok(()) - }, + } } } -fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statement: &Statement<'tcx>) -> McfResult { +fn check_statement( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + def_id: DefId, + statement: &Statement<'tcx>, +) -> McfResult { let span = statement.source_info.span; match &statement.kind { StatementKind::Assign(box (place, rval)) => { @@ -228,7 +260,12 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen } } -fn check_operand(tcx: TyCtxt<'tcx>, operand: &Operand<'tcx>, span: Span, body: &Body<'tcx>) -> McfResult { +fn check_operand( + tcx: TyCtxt<'tcx>, + operand: &Operand<'tcx>, + span: Span, + body: &Body<'tcx>, +) -> McfResult { match operand { Operand::Move(place) | Operand::Copy(place) => check_place(tcx, *place, span, body), Operand::Constant(c) => match c.check_static_ptr(tcx) { @@ -251,19 +288,23 @@ fn check_place(tcx: TyCtxt<'tcx>, place: Place<'tcx>, span: Span, body: &Body<'t return Err((span, "accessing union fields is unstable".into())); } } - }, + } ProjectionElem::ConstantIndex { .. } | ProjectionElem::Downcast(..) | ProjectionElem::Subslice { .. } | ProjectionElem::Deref - | ProjectionElem::Index(_) => {}, + | ProjectionElem::Index(_) => {} } } Ok(()) } -fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Terminator<'tcx>) -> McfResult { +fn check_terminator( + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, + terminator: &Terminator<'tcx>, +) -> McfResult { let span = terminator.source_info.span; match &terminator.kind { TerminatorKind::FalseEdge { .. } @@ -277,18 +318,16 @@ fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Termin TerminatorKind::DropAndReplace { place, value, .. } => { check_place(tcx, *place, span, body)?; check_operand(tcx, value, span, body) - }, + } - TerminatorKind::SwitchInt { - discr, - switch_ty: _, - targets: _, - } => check_operand(tcx, discr, span, body), + TerminatorKind::SwitchInt { discr, switch_ty: _, targets: _ } => { + check_operand(tcx, discr, span, body) + } TerminatorKind::Abort => Err((span, "abort is not stable in const fn".into())), TerminatorKind::GeneratorDrop | TerminatorKind::Yield { .. } => { Err((span, "const fn generators are unstable".into())) - }, + } TerminatorKind::Call { func, @@ -316,7 +355,9 @@ fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Termin // within const fns. `transmute` is allowed in all other const contexts. // This won't really scale to more intrinsics or functions. Let's allow const // transmutes in const fn before we add more hacks to this. - if tcx.fn_sig(fn_def_id).abi() == RustIntrinsic && tcx.item_name(fn_def_id) == sym::transmute { + if tcx.fn_sig(fn_def_id).abi() == RustIntrinsic + && tcx.item_name(fn_def_id) == sym::transmute + { return Err(( span, "can only call `transmute` from const items, not `const fn`".into(), @@ -332,16 +373,14 @@ fn check_terminator(tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, terminator: &Termin } else { Err((span, "can only call other const fns within const fn".into())) } - }, + } - TerminatorKind::Assert { - cond, - expected: _, - msg: _, - target: _, - cleanup: _, - } => check_operand(tcx, cond, span, body), + TerminatorKind::Assert { cond, expected: _, msg: _, target: _, cleanup: _ } => { + check_operand(tcx, cond, span, body) + } - TerminatorKind::InlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), + TerminatorKind::InlineAsm { .. } => { + Err((span, "cannot use inline assembly in const fn".into())) + } } } From b37daabed52be2ba3909821f212ed303642f6be3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 21 Nov 2020 15:33:36 -0500 Subject: [PATCH 07/16] move the `sub-unify` check and extend the documentation a bit I didn't like the sub-unify code executing when a predicate was ENQUEUED, that felt fragile. I would have preferred to move the sub-unify code so that it only occurred during generalization, but that impacted diagnostics, so having it also occur when we process subtype predicates felt pretty reasonable. (I guess we only need one or the other, but I kind of prefer both, since the generalizer ultimately feels like the *right* place to guarantee the properties we want.) --- compiler/rustc_infer/src/infer/combine.rs | 5 +++ compiler/rustc_infer/src/infer/mod.rs | 35 ++++++++++--------- compiler/rustc_infer/src/infer/sub.rs | 3 +- .../rustc_infer/src/infer/type_variable.rs | 24 ++++++++++--- 4 files changed, 45 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_infer/src/infer/combine.rs b/compiler/rustc_infer/src/infer/combine.rs index fb9cd4457f209..ca52f2f2ac4e9 100644 --- a/compiler/rustc_infer/src/infer/combine.rs +++ b/compiler/rustc_infer/src/infer/combine.rs @@ -641,6 +641,11 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { .type_variables() .new_var(self.for_universe, Diverging::NotDiverging, origin); let u = self.tcx().mk_ty_var(new_var_id); + + // Record that we replaced `vid` with `new_var_id` as part of a generalization + // operation. This is needed to detect cyclic types. To see why, see the + // docs in the `type_variables` module. + self.infcx.inner.borrow_mut().type_variables().sub(vid, new_var_id); debug!("generalize: replacing original vid={:?} with new={:?}", vid, u); Ok(u) } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 5a978a819334f..a67d43e43c4d4 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -961,23 +961,26 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { param_env: ty::ParamEnv<'tcx>, predicate: ty::PolySubtypePredicate<'tcx>, ) -> Option> { - // Subtle: it's ok to skip the binder here and resolve because - // `shallow_resolve` just ignores anything that is not a type - // variable, and because type variable's can't (at present, at - // least) capture any of the things bound by this binder. + // Check for two unresolved inference variables, in which case we can + // make no progress. This is partly a micro-optimization, but it's + // also an opportunity to "sub-unify" the variables. This isn't + // *necessary* to prevent cycles, because they would eventually be sub-unified + // anyhow during generalization, but it helps with diagnostics (we can detect + // earlier that they are sub-unified). // - // NOTE(nmatsakis): really, there is no *particular* reason to do this - // `shallow_resolve` here except as a micro-optimization. - // Naturally I could not resist. - let two_unbound_type_vars = { - let a = self.shallow_resolve(predicate.skip_binder().a); - let b = self.shallow_resolve(predicate.skip_binder().b); - a.is_ty_var() && b.is_ty_var() - }; - - if two_unbound_type_vars { - // Two unbound type variables? Can't make progress. - return None; + // Note that we can just skip the binders here because + // type variables can't (at present, at + // least) capture any of the things bound by this binder. + { + let r_a = self.shallow_resolve(predicate.skip_binder().a); + let r_b = self.shallow_resolve(predicate.skip_binder().b); + match (r_a.kind(), r_b.kind()) { + (&ty::Infer(ty::TyVar(a_vid)), &ty::Infer(ty::TyVar(b_vid))) => { + self.inner.borrow_mut().type_variables().sub(a_vid, b_vid); + return None; + } + _ => {} + } } Some(self.commit_if_ok(|_snapshot| { diff --git a/compiler/rustc_infer/src/infer/sub.rs b/compiler/rustc_infer/src/infer/sub.rs index a676c5e65a73b..a61c472494046 100644 --- a/compiler/rustc_infer/src/infer/sub.rs +++ b/compiler/rustc_infer/src/infer/sub.rs @@ -84,7 +84,7 @@ impl TypeRelation<'tcx> for Sub<'combine, 'infcx, 'tcx> { let a = infcx.inner.borrow_mut().type_variables().replace_if_possible(a); let b = infcx.inner.borrow_mut().type_variables().replace_if_possible(b); match (a.kind(), b.kind()) { - (&ty::Infer(TyVar(a_vid)), &ty::Infer(TyVar(b_vid))) => { + (&ty::Infer(TyVar(_)), &ty::Infer(TyVar(_))) => { // Shouldn't have any LBR here, so we can safely put // this under a binder below without fear of accidental // capture. @@ -96,7 +96,6 @@ impl TypeRelation<'tcx> for Sub<'combine, 'infcx, 'tcx> { // have to record in the `type_variables` tracker that // the two variables are equal modulo subtyping, which // is important to the occurs check later on. - infcx.inner.borrow_mut().type_variables().sub(a_vid, b_vid); self.fields.obligations.push(Obligation::new( self.fields.trace.cause.clone(), self.fields.param_env, diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index 6cf5bc8022e69..ca878d9c566f1 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -75,14 +75,30 @@ pub struct TypeVariableStorage<'tcx> { /// ?1 <: ?3 /// Box <: ?1 /// - /// This works because `?1` and `?3` are unified in the - /// `sub_relations` relation (not in `eq_relations`). Then when we - /// process the `Box <: ?1` constraint, we do an occurs check - /// on `Box` and find a potential cycle. + /// Without this second table, what would happen in a case like + /// this is that we would instantiate `?1` with a generalized + /// type like `Box`. We would then relate `Box <: Box` + /// and infer that `?3 <: ?6`. Next, since `?1` was instantiated, + /// we would process `?1 <: ?3`, generalize `?1 = Box` to `Box`, + /// and instantiate `?3` with `Box`. Finally, we would relate + /// `?6 <: ?9`. But now that we instantiated `?3`, we can process + /// `?3 <: ?6`, which gives us `Box <: ?6`... and the cycle + /// continues. (This is `occurs-check-2.rs`.) + /// + /// What prevents this cycle is that when we generalize + /// `Box` to `Box`, we also sub-unify `?3` and `?6` + /// (in the generalizer). When we then process `Box <: ?3`, + /// the occurs check then fails because `?6` and `?3` are sub-unified, + /// and hence generalization fails. /// /// This is reasonable because, in Rust, subtypes have the same /// "skeleton" and hence there is no possible type such that /// (e.g.) `Box <: ?3` for any `?3`. + /// + /// In practice, we sometimes sub-unify variables in other spots, such + /// as when processing subtype predicates. This is not necessary but is + /// done to aid diagnostics, as it allows us to be more effective when + /// we guide the user towards where they should insert type hints. sub_relations: ut::UnificationTableStorage, } From a4f357d3ce1e24237d7b383384a11b74bc80dec2 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 21 Nov 2020 15:47:14 -0500 Subject: [PATCH 08/16] create `Coercion` obligations given 2 unbound type variables Motivation: in upcoming commits, we are going to create a graph of the coercion relationships between variables. We want to distinguish *coercion* specifically from other sorts of subtyping, as it indicates values flowing from one place to another via assignment. --- compiler/rustc_typeck/src/check/coercion.rs | 62 ++++++++++++++++++--- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index d6cd2197d9a8f..be3002341ade7 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -40,16 +40,22 @@ use crate::check::FnCtxt; use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::DefId; -use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{Coercion, InferOk, InferResult}; -use rustc_middle::ty::adjustment::{ - Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast, +use rustc_infer::{ + infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}, + traits::Obligation, }; use rustc_middle::ty::error::TypeError; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::relate::RelateResult; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Ty, TypeAndMut}; +use rustc_middle::ty::{ + adjustment::{ + Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCast, + }, + ToPredicate, +}; use rustc_session::parse::feature_err; use rustc_span::symbol::sym; use rustc_span::{self, BytePos, Span}; @@ -171,7 +177,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { kind: TypeVariableOriginKind::AdjustmentType, span: self.cause.span, }); - self.unify_and(&b, &diverging_ty, simple(Adjust::NeverToAny)) + self.coerce_from_inference_variable(diverging_ty, b, simple(Adjust::NeverToAny)) } else { success(simple(Adjust::NeverToAny)(b), b, vec![]) }; @@ -181,7 +187,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { // we have no information about the source type. This will always // ultimately fall back to some form of subtyping. if a.is_ty_var() { - return self.coerce_from_inference_variable(a, b); + return self.coerce_from_inference_variable(a, b, identity); } // Consider coercing the subtype to a DST @@ -244,11 +250,53 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { /// Coercing *from* an inference variable. In this case, we have no information /// about the source type, so we can't really do a true coercion and we always /// fall back to subtyping (`unify_and`). - fn coerce_from_inference_variable(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> CoerceResult<'tcx> { + fn coerce_from_inference_variable( + &self, + a: Ty<'tcx>, + b: Ty<'tcx>, + make_adjustments: impl FnOnce(Ty<'tcx>) -> Vec>, + ) -> CoerceResult<'tcx> { + debug!("coerce_from_inference_variable(a={:?}, b={:?})", a, b); assert!(a.is_ty_var() && self.infcx.shallow_resolve(a) == a); assert!(self.infcx.shallow_resolve(b) == b); - self.unify_and(a, b, identity) + if b.is_ty_var() { + // Two unresolved type variables: create a `Coerce` predicate. + let target_ty = if self.use_lub { + self.infcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::LatticeVariable, + span: self.cause.span, + }) + } else { + b + }; + + let mut obligations = Vec::with_capacity(2); + for &source_ty in &[a, b] { + if source_ty != target_ty { + obligations.push(Obligation::new( + self.cause.clone(), + self.param_env, + ty::PredicateAtom::Coerce(ty::CoercePredicate { + a: source_ty, + b: target_ty, + }) + .to_predicate(self.tcx()), + )); + } + } + + debug!( + "coerce_from_inference_variable: two inference variables, target_ty={:?}, obligations={:?}", + target_ty, obligations + ); + let adjustments = make_adjustments(target_ty); + InferResult::Ok(InferOk { value: (adjustments, target_ty), obligations }) + } else { + // One unresolved type variable: just apply subtyping, we may be able + // to do something useful. + self.unify_and(a, b, make_adjustments) + } } /// Reborrows `&mut A` to `&mut B` and `&(mut) A` to `&B`. From de3bf35ab3b51666e4fbc07eb8aeaea63cb3cbea Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 21 Nov 2020 16:39:07 -0500 Subject: [PATCH 09/16] make `TyVid` implement the `Idx` trait --- compiler/rustc_infer/src/infer/fudge.rs | 2 +- .../rustc_infer/src/infer/type_variable.rs | 20 +++++++++---------- compiler/rustc_middle/src/infer/unify_key.rs | 4 ++-- .../rustc_middle/src/ty/structural_impls.rs | 2 +- compiler/rustc_middle/src/ty/sty.rs | 7 ++++--- .../src/traits/error_reporting/suggestions.rs | 4 ++-- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_infer/src/infer/fudge.rs b/compiler/rustc_infer/src/infer/fudge.rs index c292b2bdb3040..c44f7a67c3ded 100644 --- a/compiler/rustc_infer/src/infer/fudge.rs +++ b/compiler/rustc_infer/src/infer/fudge.rs @@ -187,7 +187,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for InferenceFudger<'a, 'tcx> { if self.type_vars.0.contains(&vid) { // This variable was created during the fudging. // Recreate it with a fresh variable here. - let idx = (vid.index - self.type_vars.0.start.index) as usize; + let idx = vid.as_usize() - self.type_vars.0.start.as_usize(); let origin = self.type_vars.1[idx]; self.infcx.next_ty_var(origin) } else { diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index ca878d9c566f1..914ca94579e39 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -198,7 +198,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { /// Note that this function does not return care whether /// `vid` has been unified with something else or not. pub fn var_diverges(&self, vid: ty::TyVid) -> Diverging { - self.storage.values.get(vid.index as usize).diverging + self.storage.values.get(vid.as_usize()).diverging } /// Returns the origin that was given when `vid` was created. @@ -206,7 +206,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { /// Note that this function does not return care whether /// `vid` has been unified with something else or not. pub fn var_origin(&self, vid: ty::TyVid) -> &TypeVariableOrigin { - &self.storage.values.get(vid.index as usize).origin + &self.storage.values.get(vid.as_usize()).origin } /// Records that `a == b`, depending on `dir`. @@ -271,7 +271,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { assert_eq!(eq_key.vid, sub_key); let index = self.values().push(TypeVariableData { origin, diverging }); - assert_eq!(eq_key.vid.index, index as u32); + assert_eq!(eq_key.vid.as_usize(), index); debug!( "new_var(index={:?}, universe={:?}, diverging={:?}, origin={:?}", @@ -359,11 +359,11 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { &mut self, value_count: usize, ) -> (Range, Vec) { - let range = TyVid { index: value_count as u32 }..TyVid { index: self.num_vars() as u32 }; + let range = TyVid::from(value_count)..TyVid::from(self.num_vars()); ( range.start..range.end, - (range.start.index..range.end.index) - .map(|index| self.storage.values.get(index as usize).origin) + (range.start.as_usize()..range.end.as_usize()) + .map(|index| self.storage.values.get(index).origin) .collect(), ) } @@ -395,7 +395,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { super::UndoLog::TypeVariables(UndoLog::Values(sv::UndoLog::Other( Instantiate { vid, .. }, ))) => { - if vid.index < new_elem_threshold { + if vid.as_u32() < new_elem_threshold { // quick check to see if this variable was // created since the snapshot started or not. let mut eq_relations = ut::UnificationTable::with_log( @@ -423,7 +423,7 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { pub fn unsolved_variables(&mut self) -> Vec { (0..self.storage.values.len()) .filter_map(|i| { - let vid = ty::TyVid { index: i as u32 }; + let vid = ty::TyVid::from(i); match self.probe(vid) { TypeVariableValue::Unknown { .. } => Some(vid), TypeVariableValue::Known { .. } => None, @@ -475,10 +475,10 @@ impl<'tcx> From for TyVidEqKey<'tcx> { impl<'tcx> ut::UnifyKey for TyVidEqKey<'tcx> { type Value = TypeVariableValue<'tcx>; fn index(&self) -> u32 { - self.vid.index + self.vid.as_u32() } fn from_index(i: u32) -> Self { - TyVidEqKey::from(ty::TyVid { index: i }) + TyVidEqKey::from(ty::TyVid::from(i)) } fn tag() -> &'static str { "TyVidEqKey" diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index 16e9aafb25a54..24ad3ab9ccdf6 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -20,10 +20,10 @@ pub trait ToType { impl UnifyKey for ty::TyVid { type Value = (); fn index(&self) -> u32 { - self.index + u32::from(*self) } fn from_index(i: u32) -> ty::TyVid { - ty::TyVid { index: i } + ty::TyVid::from(i) } fn tag() -> &'static str { "TyVid" diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs index 713191264c7d5..8a26a5171e27a 100644 --- a/compiler/rustc_middle/src/ty/structural_impls.rs +++ b/compiler/rustc_middle/src/ty/structural_impls.rs @@ -130,7 +130,7 @@ impl fmt::Debug for ty::FnSig<'tcx> { impl fmt::Debug for ty::TyVid { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "_#{}t", self.index) + write!(f, "_#{}t", u32::from(*self)) } } diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 4bf16436855f4..12e2733064e34 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1489,9 +1489,10 @@ pub struct EarlyBoundRegion { pub name: Symbol, } -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, TyEncodable, TyDecodable)] -pub struct TyVid { - pub index: u32, +rustc_index::newtype_index! { + pub struct TyVid { + DEBUG_FORMAT = custom, + } } #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, TyEncodable, TyDecodable)] diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 095483aa5a25d..a4936b7e48518 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1157,7 +1157,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let sig = if let ty::Tuple(inputs) = inputs.kind() { tcx.mk_fn_sig( inputs.iter().map(|k| k.expect_ty()), - tcx.mk_ty_infer(ty::TyVar(ty::TyVid { index: 0 })), + tcx.mk_ty_infer(ty::TyVar(ty::TyVid::from(0_u32))), false, hir::Unsafety::Normal, abi::Abi::Rust, @@ -1165,7 +1165,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } else { tcx.mk_fn_sig( std::iter::once(inputs), - tcx.mk_ty_infer(ty::TyVar(ty::TyVid { index: 0 })), + tcx.mk_ty_infer(ty::TyVar(ty::TyVid::from(0_u32))), false, hir::Unsafety::Normal, abi::Abi::Rust, From 73b743b92f198a395b35ec5bf4754dee75b29318 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 22 Nov 2020 06:10:50 -0500 Subject: [PATCH 10/16] move `fallback_if_possible` and friends to fallback.rs Along the way, simplify and document the logic more clearly. --- compiler/rustc_infer/src/infer/mod.rs | 38 +++--- compiler/rustc_middle/src/ty/error.rs | 6 - compiler/rustc_typeck/src/check/fallback.rs | 110 +++++++++++++++++- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 87 +------------- compiler/rustc_typeck/src/check/mod.rs | 10 -- 5 files changed, 125 insertions(+), 126 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index a67d43e43c4d4..d80d14c459734 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -21,7 +21,7 @@ use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKin use rustc_middle::mir; use rustc_middle::mir::interpret::EvalToConstValueResult; use rustc_middle::traits::select; -use rustc_middle::ty::error::{ExpectedFound, TypeError, UnconstrainedNumeric}; +use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::fold::{TypeFoldable, TypeFolder}; use rustc_middle::ty::relate::RelateResult; use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts, SubstsRef}; @@ -641,6 +641,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { t.fold_with(&mut self.freshener()) } + /// Returns whether `ty` is a diverging type variable or not. + /// (If `ty` is not a type variable at all, returns not diverging.) + /// + /// 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), @@ -648,32 +652,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } - pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> { - freshen::TypeFreshener::new(self) - } - - pub fn type_is_unconstrained_numeric(&'a self, ty: Ty<'_>) -> UnconstrainedNumeric { - use rustc_middle::ty::error::UnconstrainedNumeric::Neither; - use rustc_middle::ty::error::UnconstrainedNumeric::{UnconstrainedFloat, UnconstrainedInt}; + /// Returns the origin of the type variable identified by `vid`, or `None` + /// if this is not a type variable. + /// + /// No attempt is made to resolve `ty`. + pub fn type_var_origin(&'a self, ty: Ty<'tcx>) -> Option { match *ty.kind() { - ty::Infer(ty::IntVar(vid)) => { - if self.inner.borrow_mut().int_unification_table().probe_value(vid).is_some() { - Neither - } else { - UnconstrainedInt - } + ty::Infer(ty::TyVar(vid)) => { + Some(*self.inner.borrow_mut().type_variables().var_origin(vid)) } - ty::Infer(ty::FloatVar(vid)) => { - if self.inner.borrow_mut().float_unification_table().probe_value(vid).is_some() { - Neither - } else { - UnconstrainedFloat - } - } - _ => Neither, + _ => None, } } + pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> { + freshen::TypeFreshener::new(self) + } pub fn unsolved_variables(&self) -> Vec> { let mut inner = self.inner.borrow_mut(); let mut vars: Vec> = inner diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index 5ec0ec0c56ad6..7cf1d3b63adb1 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -67,12 +67,6 @@ pub enum TypeError<'tcx> { TargetFeatureCast(DefId), } -pub enum UnconstrainedNumeric { - UnconstrainedFloat, - UnconstrainedInt, - Neither, -} - /// Explains the source of a type err in a short, human readable way. This is meant to be placed /// in parentheses after some larger message. You should also invoke `note_and_explain_type_err()` /// afterwards to present additional details, particularly when it comes to lifetime-related diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index 5d884f1f54691..c52841b48bf53 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -1,5 +1,6 @@ -use crate::check::FallbackMode; use crate::check::FnCtxt; +use rustc_infer::infer::type_variable::Diverging; +use rustc_middle::ty::{self, Ty}; impl<'tcx> FnCtxt<'_, 'tcx> { pub(super) fn type_inference_fallback(&self) { @@ -12,8 +13,9 @@ impl<'tcx> FnCtxt<'_, 'tcx> { // The first time, we do *not* replace opaque types. for ty in &self.unsolved_variables() { debug!("unsolved_variable = {:?}", ty); - fallback_has_occurred |= self.fallback_if_possible(ty, FallbackMode::NoOpaque); + fallback_has_occurred |= self.fallback_if_possible(ty); } + // 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 @@ -43,10 +45,112 @@ impl<'tcx> FnCtxt<'_, 'tcx> { // unconstrained opaque type variables, in addition to performing // other kinds of fallback. for ty in &self.unsolved_variables() { - fallback_has_occurred |= self.fallback_if_possible(ty, FallbackMode::All); + fallback_has_occurred |= self.fallback_opaque_type_vars(ty); } // See if we can make any more progress. self.select_obligations_where_possible(fallback_has_occurred, |_| {}); } + + // Tries to apply a fallback to `ty` if it is an unsolved variable. + // + // - Unconstrained ints are replaced with `i32`. + // + // - Unconstrained floats are replaced with with `f64`. + // + // - Non-numerics get replaced with `!` when `#![feature(never_type_fallback)]` + // is enabled. Otherwise, they are replaced with `()`. + // + // 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 { + // 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. + // + // 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.) + 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, + }, + }; + debug!("fallback_if_possible(ty={:?}): defaulting to `{:?}`", ty, fallback); + + let span = self + .infcx + .type_var_origin(ty) + .map(|origin| origin.span) + .unwrap_or(rustc_span::DUMMY_SP); + self.demand_eqtype(span, ty, fallback); + 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. + /// + /// For example, in this code: + /// + ///``` + /// type MyType = impl Copy; + /// fn defining_use() -> MyType { true } + /// fn other_use() -> MyType { defining_use() } + /// ``` + /// + /// `defining_use` will constrain the instantiated inference + /// variable to `bool`, while `other_use` will constrain + /// the instantiated inference variable to `MyType`. + /// + /// When we process opaque types during writeback, we + /// will handle cases like `other_use`, and not count + /// them as defining usages + /// + /// However, we also need to handle cases like this: + /// + /// ```rust + /// pub type Foo = impl Copy; + /// fn produce() -> Option { + /// None + /// } + /// ``` + /// + /// In the above snippet, the inference variable created by + /// instantiating `Option` will be completely unconstrained. + /// We treat this as a non-defining use by making the inference + /// variable fall back to the opaque type itself. + fn fallback_opaque_type_vars(&self, ty: Ty<'tcx>) -> bool { + let span = self + .infcx + .type_var_origin(ty) + .map(|origin| origin.span) + .unwrap_or(rustc_span::DUMMY_SP); + if let Some(&opaque_ty) = self.opaque_types_vars.borrow().get(ty) { + debug!( + "fallback_opaque_type_vars(ty={:?}): falling back to opaque type {:?}", + ty, opaque_ty + ); + self.demand_eqtype(span, ty, opaque_ty); + true + } else { + return false; + } + } } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index c6c73e793be2e..5cef3c02ac8dd 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -4,7 +4,7 @@ use crate::astconv::{ }; use crate::check::callee::{self, DeferredCallResolution}; use crate::check::method::{self, MethodCallee, SelfSource}; -use crate::check::{BreakableCtxt, Diverges, Expectation, FallbackMode, FnCtxt, LocalTy}; +use crate::check::{BreakableCtxt, Diverges, Expectation, FnCtxt, LocalTy}; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; @@ -14,11 +14,8 @@ use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_hir::{ExprKind, GenericArg, Node, QPath}; +use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryResponse}; use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282; -use rustc_infer::infer::{ - canonical::{Canonical, OriginalQueryValues, QueryResponse}, - type_variable::Diverging, -}; use rustc_infer::infer::{InferOk, InferResult}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::fold::TypeFoldable; @@ -612,86 +609,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - // Tries to apply a fallback to `ty` if it is an unsolved variable. - // - // - Unconstrained ints are replaced with `i32`. - // - // - Unconstrained floats are replaced with with `f64`. - // - // - Non-numerics get replaced with `!` when `#![feature(never_type_fallback)]` - // is enabled. Otherwise, they are replaced with `()`. - // - // 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. - pub(in super::super) fn fallback_if_possible(&self, ty: Ty<'tcx>, mode: FallbackMode) -> bool { - use rustc_middle::ty::error::UnconstrainedNumeric::Neither; - use rustc_middle::ty::error::UnconstrainedNumeric::{UnconstrainedFloat, UnconstrainedInt}; - - assert!(ty.is_ty_infer()); - let fallback = match self.type_is_unconstrained_numeric(ty) { - _ if self.is_tainted_by_errors() => self.tcx().ty_error(), - UnconstrainedInt => self.tcx.types.i32, - UnconstrainedFloat => self.tcx.types.f64, - Neither => match self.type_var_diverges(ty) { - Diverging::Diverges => self.tcx.mk_diverging_default(), - - Diverging::NotDiverging => { - // This type variable was created from the instantiation of an opaque - // type. The fact that we're attempting to perform fallback for it - // means that the function neither constrained it to a concrete - // type, nor to the opaque type itself. - // - // For example, in this code: - // - //``` - // type MyType = impl Copy; - // fn defining_use() -> MyType { true } - // fn other_use() -> MyType { defining_use() } - // ``` - // - // `defining_use` will constrain the instantiated inference - // variable to `bool`, while `other_use` will constrain - // the instantiated inference variable to `MyType`. - // - // When we process opaque types during writeback, we - // will handle cases like `other_use`, and not count - // them as defining usages - // - // However, we also need to handle cases like this: - // - // ```rust - // pub type Foo = impl Copy; - // fn produce() -> Option { - // None - // } - // ``` - // - // In the above snippet, the inference variable created by - // instantiating `Option` will be completely unconstrained. - // We treat this as a non-defining use by making the inference - // variable fall back to the opaque type itself. - if let FallbackMode::All = mode { - if let Some(opaque_ty) = self.opaque_types_vars.borrow().get(ty) { - debug!( - "fallback_if_possible: falling back opaque type var {:?} to {:?}", - ty, opaque_ty - ); - *opaque_ty - } else { - return false; - } - } else { - return false; - } - } - }, - }; - debug!("fallback_if_possible: defaulting `{:?}` to `{:?}`", ty, fallback); - self.demand_eqtype(rustc_span::DUMMY_SP, ty, fallback); - true - } - pub(in super::super) fn select_all_obligations_or_error(&self) { debug!("select_all_obligations_or_error"); if let Err(errors) = self.fulfillment_cx.borrow_mut().select_all_or_error(&self) { diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 7547b98a740ef..943ca1cbefc3a 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -1046,16 +1046,6 @@ enum TupleArgumentsFlag { TupleArguments, } -/// Controls how we perform fallback for unconstrained -/// type variables. -enum FallbackMode { - /// Do not fallback type variables to opaque types. - NoOpaque, - /// Perform all possible kinds of fallback, including - /// turning type variables to opaque types. - All, -} - /// A wrapper for `InferCtxt`'s `in_progress_typeck_results` field. #[derive(Copy, Clone)] struct MaybeInProgressTables<'a, 'tcx> { From 694d3c86557e2304c7bc97b2050c19d0e660000c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 22 Nov 2020 09:58:58 -0500 Subject: [PATCH 11/16] 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 | 13 +- compiler/rustc_middle/src/ty/sty.rs | 8 + compiler/rustc_typeck/src/check/fallback.rs | 273 +++++++++++++++--- .../diverging-fallback-control-flow.rs | 35 +-- .../never_type/diverging-fallback-no-leak.rs | 15 + .../diverging-fallback-no-leak.stderr | 14 + ...diverging-fallback-unconstrained-return.rs | 34 +++ 7 files changed, 338 insertions(+), 54 deletions(-) create mode 100644 src/test/ui/never_type/diverging-fallback-no-leak.rs create mode 100644 src/test/ui/never_type/diverging-fallback-no-leak.stderr create mode 100644 src/test/ui/never_type/diverging-fallback-unconstrained-return.rs diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index d80d14c459734..c2176fcdbcd4f 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -647,11 +647,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. /// @@ -1003,6 +1009,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) } diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 12e2733064e34..708a498bfdae6 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1902,6 +1902,14 @@ impl<'tcx> TyS<'tcx> { matches!(self.kind(), Infer(TyVar(_))) } + #[inline] + pub fn ty_vid(&self) -> Option { + match self.kind() { + &Infer(TyVar(vid)) => Some(vid), + _ => None, + } + } + #[inline] pub fn is_ty_infer(&self) -> bool { matches!(self.kind(), Infer(_)) diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index c52841b48bf53..123a9ca349e4f 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/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,20 +11,28 @@ impl<'tcx> FnCtxt<'_, 'tcx> { 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; + } + + let diverging_fallback = self.calculate_diverging_fallback(&unsolved_variables); + // 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: // @@ -58,36 +69,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>>, + ) -> 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); @@ -101,11 +120,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: /// @@ -153,4 +171,191 @@ 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>> { + 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::>() + ); + 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 { + 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.bound_atom().no_bound_vars() + }) + .filter_map(|atom| { + if let ty::PredicateAtom::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 { + None + } + }) + .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 { + Some(self.infcx.root_var(self.infcx.shallow_resolve(ty).ty_vid()?)) + } } diff --git a/src/test/ui/never_type/diverging-fallback-control-flow.rs b/src/test/ui/never_type/diverging-fallback-control-flow.rs index ea4881049d792..f323f40ba31c6 100644 --- a/src/test/ui/never_type/diverging-fallback-control-flow.rs +++ b/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. -// -// These represent current behavior, but are pretty dubious. I would -// like to revisit these and potentially change them. --nmatsakis - +// to fallback based on control-flow. In all of these cases, +// the type variable winds up being the target of both a `!` coercion +// and a coercion from a non-`!` variable, and hence falls back to `()`. #![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 ! { - fn default() -> ! { +impl UnitDefault for () { + 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,9 +91,9 @@ fn loop_break_rev() { if false { break return; } else { - break BadDefault::default(); + break UnitDefault::default(); } }; } -fn main() { } +fn main() {} diff --git a/src/test/ui/never_type/diverging-fallback-no-leak.rs b/src/test/ui/never_type/diverging-fallback-no-leak.rs new file mode 100644 index 0000000000000..a3a15f0ed885d --- /dev/null +++ b/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) {} + +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 +} diff --git a/src/test/ui/never_type/diverging-fallback-no-leak.stderr b/src/test/ui/never_type/diverging-fallback-no-leak.stderr new file mode 100644 index 0000000000000..9fcddc654610e --- /dev/null +++ b/src/test/ui/never_type/diverging-fallback-no-leak.stderr @@ -0,0 +1,14 @@ +error[E0277]: the trait bound `!: Test` is not satisfied + --> $DIR/diverging-fallback-no-leak.rs:14:5 + | +LL | fn unconstrained_arg(_: T) {} + | ---- required by this bound in `unconstrained_arg` +... +LL | unconstrained_arg(return); + | ^^^^^^^^^^^^^^^^^ the trait `Test` is not implemented for `!` + | + = note: the trait is implemented for `()`. Possibly this error has been caused by changes to Rust's type-inference algorithm (see issue #48950 for more information). Consider whether you meant to use the type `()` here instead. + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/never_type/diverging-fallback-unconstrained-return.rs b/src/test/ui/never_type/diverging-fallback-unconstrained-return.rs new file mode 100644 index 0000000000000..9a6c965cefb09 --- /dev/null +++ b/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 { + 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!() }; +} From 8eda90e84bca54718a1bb2fb46e88e1e4be404b4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Nov 2020 07:42:06 -0500 Subject: [PATCH 12/16] remove diverging type variables from fn check The comment seems incorrect. Testing revealed that the examples in question still work (as well as some variants) even without the special casing here. --- .../rustc_infer/src/infer/type_variable.rs | 6 +++- compiler/rustc_typeck/src/check/check.rs | 32 +++++-------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index 914ca94579e39..b2fa613addfc7 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -129,7 +129,11 @@ pub enum TypeVariableOriginKind { SubstitutionPlaceholder, AutoDeref, AdjustmentType, - DivergingFn, + + /// In type check, when we are type checking a function that + /// returns `-> dyn Foo`, we substitute a type variable for the + /// return type for diagnostic purposes. + DynReturnFn, LatticeVariable, } diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index d5518dfc15a1b..5f30b8fc7094f 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -218,32 +218,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 = 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); From fceb5234fa64e50f44daed40fc04e7b3fef449d1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Nov 2020 07:43:33 -0500 Subject: [PATCH 13/16] remove reliance on "diverging" type variables Instead, we now record those type variables that are the target of a `NeverToAny` adjustment and consider those to be the "diverging" type variables. This allows us to remove the special case logic that creates a type variable for `!` in coercion. --- compiler/rustc_typeck/src/check/coercion.rs | 19 +------ compiler/rustc_typeck/src/check/fallback.rs | 57 ++++++++++++------- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 9 +++ compiler/rustc_typeck/src/check/inherited.rs | 8 ++- 4 files changed, 53 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index be3002341ade7..84a486250990b 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -163,24 +163,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 = 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 diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index 123a9ca349e4f..834827633aad5 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -2,7 +2,6 @@ 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}; impl<'tcx> FnCtxt<'_, 'tcx> { @@ -250,8 +249,27 @@ impl<'tcx> FnCtxt<'_, 'tcx> { // 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(); + 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 = 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 @@ -260,27 +278,24 @@ impl<'tcx> FnCtxt<'_, 'tcx> { 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 { + for unsolved_vid in unsolved_vids { + let root_vid = self.infcx.root_var(unsolved_vid); debug!( - "calculate_diverging_fallback: unsolved_vid={:?} diverges={:?}", + "calculate_diverging_fallback: unsolved_vid={:?} root_vid={:?} diverges={:?}", unsolved_vid, - self.infcx.ty_vid_diverges(unsolved_vid) + root_vid, + diverging_roots.contains(&root_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::>() - ); - roots_reachable_from_diverging - .extend(coercion_graph.depth_first_search(root_vid)); - } - Diverging::NotDiverging => { - non_diverging_vids.push(unsolved_vid); - } + if diverging_roots.contains(&root_vid) { + diverging_vids.push(unsolved_vid); + debug!( + "calculate_diverging_fallback: root_vid={:?} reaches {:?}", + root_vid, + coercion_graph.depth_first_search(root_vid).collect::>() + ); + roots_reachable_from_diverging.extend(coercion_graph.depth_first_search(root_vid)); + } else { + non_diverging_vids.push(unsolved_vid); } } debug!( diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index 5cef3c02ac8dd..af510d6d9f74e 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -273,6 +273,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, &Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Mut { .. })), diff --git a/compiler/rustc_typeck/src/check/inherited.rs b/compiler/rustc_typeck/src/check/inherited.rs index 0011a3fc71b59..5cee8d1f5d9fa 100644 --- a/compiler/rustc_typeck/src/check/inherited.rs +++ b/compiler/rustc_typeck/src/check/inherited.rs @@ -1,7 +1,7 @@ use super::callee::DeferredCallResolution; use super::MaybeInProgressTables; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet}; use rustc_hir as hir; use rustc_hir::def_id::{DefIdMap, LocalDefId}; use rustc_hir::HirIdMap; @@ -68,6 +68,11 @@ pub struct Inherited<'a, 'tcx> { pub(super) opaque_types_vars: RefCell, Ty<'tcx>>>, pub(super) body_id: Option, + + /// 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>>, } impl<'a, 'tcx> Deref for Inherited<'a, 'tcx> { @@ -125,6 +130,7 @@ impl Inherited<'a, 'tcx> { deferred_generator_interiors: RefCell::new(Vec::new()), opaque_types: RefCell::new(Default::default()), opaque_types_vars: RefCell::new(Default::default()), + diverging_type_vars: RefCell::new(Default::default()), body_id, } } From fc02f0ff620709910584b3aef6fda256a298dd97 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Nov 2020 09:06:37 -0500 Subject: [PATCH 14/16] stop categorizing inference variables as diverging when created Instead, we now rely on the code that looks for a NeverToAny adjustment. --- compiler/rustc_infer/src/infer/combine.rs | 15 ++++---- compiler/rustc_infer/src/infer/mod.rs | 36 +++---------------- .../rustc_infer/src/infer/nll_relate/mod.rs | 4 +-- .../rustc_infer/src/infer/type_variable.rs | 23 ++---------- compiler/rustc_typeck/src/check/expr.rs | 2 +- 5 files changed, 17 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_infer/src/infer/combine.rs b/compiler/rustc_infer/src/infer/combine.rs index ca52f2f2ac4e9..c6ccd2fff8389 100644 --- a/compiler/rustc_infer/src/infer/combine.rs +++ b/compiler/rustc_infer/src/infer/combine.rs @@ -22,6 +22,7 @@ // is also useful to track which value is the "expected" value in // terms of error reporting. +use super::equate::Equate; use super::glb::Glb; use super::lub::Lub; use super::sub::Sub; @@ -29,7 +30,6 @@ use super::type_variable::TypeVariableValue; use super::unify_key::replace_if_possible; use super::unify_key::{ConstVarValue, ConstVariableValue}; use super::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; -use super::{equate::Equate, type_variable::Diverging}; use super::{InferCtxt, MiscVariable, TypeTrace}; use crate::traits::{Obligation, PredicateObligations}; @@ -639,7 +639,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { .inner .borrow_mut() .type_variables() - .new_var(self.for_universe, Diverging::NotDiverging, origin); + .new_var(self.for_universe, origin); let u = self.tcx().mk_ty_var(new_var_id); // Record that we replaced `vid` with `new_var_id` as part of a generalization @@ -861,11 +861,12 @@ impl TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> { let origin = *self.infcx.inner.borrow_mut().type_variables().var_origin(vid); - let new_var_id = self.infcx.inner.borrow_mut().type_variables().new_var( - self.for_universe, - Diverging::NotDiverging, - origin, - ); + let new_var_id = self + .infcx + .inner + .borrow_mut() + .type_variables() + .new_var(self.for_universe, origin); let u = self.tcx().mk_ty_var(new_var_id); debug!( "ConstInferUnifier: replacing original vid={:?} with new={:?}", diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index c2176fcdbcd4f..a11ff03311a8d 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -44,7 +44,7 @@ use self::region_constraints::{GenericKind, RegionConstraintData, VarInfos, Veri use self::region_constraints::{ RegionConstraintCollector, RegionConstraintStorage, RegionSnapshot, }; -use self::type_variable::{Diverging, TypeVariableOrigin, TypeVariableOriginKind}; +use self::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; pub mod at; pub mod canonical; @@ -641,23 +641,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { t.fold_with(&mut self.freshener()) } - /// Returns whether `ty` is a diverging type variable or not. - /// (If `ty` is not a type variable at all, returns not diverging.) - /// - /// 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.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. /// @@ -1014,12 +997,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { 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) + pub fn next_ty_var_id(&self, origin: TypeVariableOrigin) -> TyVid { + self.inner.borrow_mut().type_variables().new_var(self.universe(), origin) } pub fn next_ty_var(&self, origin: TypeVariableOrigin) -> Ty<'tcx> { - self.tcx.mk_ty_var(self.next_ty_var_id(Diverging::NotDiverging, origin)) + self.tcx.mk_ty_var(self.next_ty_var_id(origin)) } pub fn next_ty_var_in_universe( @@ -1027,18 +1010,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { origin: TypeVariableOrigin, universe: ty::UniverseIndex, ) -> Ty<'tcx> { - let vid = self.inner.borrow_mut().type_variables().new_var( - universe, - Diverging::NotDiverging, - origin, - ); + let vid = self.inner.borrow_mut().type_variables().new_var(universe, origin); self.tcx.mk_ty_var(vid) } - pub fn next_diverging_ty_var(&self, origin: TypeVariableOrigin) -> Ty<'tcx> { - self.tcx.mk_ty_var(self.next_ty_var_id(Diverging::Diverges, origin)) - } - pub fn next_const_var( &self, ty: Ty<'tcx>, @@ -1150,7 +1125,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // as the substitutions for the default, `(T, U)`. let ty_var_id = self.inner.borrow_mut().type_variables().new_var( self.universe(), - Diverging::NotDiverging, TypeVariableOrigin { kind: TypeVariableOriginKind::TypeParameterDefinition( param.name, diff --git a/compiler/rustc_infer/src/infer/nll_relate/mod.rs b/compiler/rustc_infer/src/infer/nll_relate/mod.rs index 18fd6f8c42706..ac04494254658 100644 --- a/compiler/rustc_infer/src/infer/nll_relate/mod.rs +++ b/compiler/rustc_infer/src/infer/nll_relate/mod.rs @@ -22,7 +22,6 @@ //! constituents) use crate::infer::combine::ConstEquateRelation; -use crate::infer::type_variable::Diverging; use crate::infer::InferCtxt; use crate::infer::{ConstVarValue, ConstVariableValue}; use rustc_data_structures::fx::FxHashMap; @@ -899,8 +898,7 @@ where // Replacing with a new variable in the universe `self.universe`, // it will be unified later with the original type variable in // the universe `_universe`. - let new_var_id = - variables.new_var(self.universe, Diverging::NotDiverging, origin); + let new_var_id = variables.new_var(self.universe, origin); let u = self.tcx().mk_ty_var(new_var_id); debug!("generalize: replacing original vid={:?} with new={:?}", vid, u); diff --git a/compiler/rustc_infer/src/infer/type_variable.rs b/compiler/rustc_infer/src/infer/type_variable.rs index b2fa613addfc7..6919a8d75c4a6 100644 --- a/compiler/rustc_infer/src/infer/type_variable.rs +++ b/compiler/rustc_infer/src/infer/type_variable.rs @@ -139,13 +139,6 @@ pub enum TypeVariableOriginKind { pub(crate) struct TypeVariableData { origin: TypeVariableOrigin, - diverging: Diverging, -} - -#[derive(Copy, Clone, Debug)] -pub enum Diverging { - NotDiverging, - Diverges, } #[derive(Copy, Clone, Debug)] @@ -197,14 +190,6 @@ impl<'tcx> TypeVariableStorage<'tcx> { } impl<'tcx> TypeVariableTable<'_, 'tcx> { - /// Returns the diverges flag given when `vid` was created. - /// - /// Note that this function does not return care whether - /// `vid` has been unified with something else or not. - pub fn var_diverges(&self, vid: ty::TyVid) -> Diverging { - self.storage.values.get(vid.as_usize()).diverging - } - /// Returns the origin that was given when `vid` was created. /// /// Note that this function does not return care whether @@ -266,7 +251,6 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { pub fn new_var( &mut self, universe: ty::UniverseIndex, - diverging: Diverging, origin: TypeVariableOrigin, ) -> ty::TyVid { let eq_key = self.eq_relations().new_key(TypeVariableValue::Unknown { universe }); @@ -274,13 +258,10 @@ impl<'tcx> TypeVariableTable<'_, 'tcx> { let sub_key = self.sub_relations().new_key(()); assert_eq!(eq_key.vid, sub_key); - let index = self.values().push(TypeVariableData { origin, diverging }); + let index = self.values().push(TypeVariableData { origin }); assert_eq!(eq_key.vid.as_usize(), index); - debug!( - "new_var(index={:?}, universe={:?}, diverging={:?}, origin={:?}", - eq_key.vid, universe, diverging, origin, - ); + debug!("new_var(index={:?}, universe={:?}, origin={:?}", eq_key.vid, universe, origin,); eq_key.vid } diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index f7f9e607a7441..08fb9f6f26ceb 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -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, }); From 128de40eee7be5e297007a1d74ee8b3eb8f40afc Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Nov 2020 09:26:10 -0500 Subject: [PATCH 15/16] fix bug in `DepthFirstSearch` where start node was not visited This could cause us to visit the start node twice. --- compiler/rustc_data_structures/src/graph/iterate/mod.rs | 5 ++++- compiler/rustc_data_structures/src/graph/iterate/tests.rs | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_data_structures/src/graph/iterate/mod.rs b/compiler/rustc_data_structures/src/graph/iterate/mod.rs index 1634c5863163b..3631f7e3f55ae 100644 --- a/compiler/rustc_data_structures/src/graph/iterate/mod.rs +++ b/compiler/rustc_data_structures/src/graph/iterate/mod.rs @@ -84,7 +84,10 @@ where G: ?Sized + DirectedGraph + WithNumNodes + WithSuccessors, { pub fn new(graph: &'graph G, start_node: G::Node) -> Self { - Self { graph, stack: vec![start_node], visited: BitSet::new_empty(graph.num_nodes()) } + let mut visited = BitSet::new_empty(graph.num_nodes()); + visited.insert(start_node); + Self { graph, stack: vec![start_node], visited } + } } } diff --git a/compiler/rustc_data_structures/src/graph/iterate/tests.rs b/compiler/rustc_data_structures/src/graph/iterate/tests.rs index 0e038e88b221d..3ee03d8b29214 100644 --- a/compiler/rustc_data_structures/src/graph/iterate/tests.rs +++ b/compiler/rustc_data_structures/src/graph/iterate/tests.rs @@ -20,3 +20,11 @@ fn is_cyclic() { assert!(!is_cyclic(&diamond_acyclic)); assert!(is_cyclic(&diamond_cyclic)); } + +#[test] +fn dfs() { + let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (2, 3), (3, 0)]); + + let result: Vec = DepthFirstSearch::new(&graph, 0).collect(); + assert_eq!(result, vec![0, 2, 3, 1]); +} From 4157667885f1c961ae9393e0cd7a639cff90e173 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Nov 2020 10:21:54 -0500 Subject: [PATCH 16/16] optimization: use a single DepthFirstSearch instead of hashsets Extend the `DepthFirstSearch` iterator so that it can be re-used and extended with add'l start nodes. Then replace the FxHashSets of nodes we were using in the fallback analysis with a single iterator. This way we won't re-walk portions of the graph that are reached more than once, and we also do less allocation etc. --- .../src/graph/iterate/mod.rs | 55 +++++++++++++++++-- .../src/graph/iterate/tests.rs | 10 +++- .../rustc_data_structures/src/graph/mod.rs | 2 +- compiler/rustc_typeck/src/check/fallback.rs | 23 +++++--- 4 files changed, 77 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_data_structures/src/graph/iterate/mod.rs b/compiler/rustc_data_structures/src/graph/iterate/mod.rs index 3631f7e3f55ae..64100fe176060 100644 --- a/compiler/rustc_data_structures/src/graph/iterate/mod.rs +++ b/compiler/rustc_data_structures/src/graph/iterate/mod.rs @@ -83,11 +83,58 @@ impl DepthFirstSearch<'graph, G> where G: ?Sized + DirectedGraph + WithNumNodes + WithSuccessors, { - pub fn new(graph: &'graph G, start_node: G::Node) -> Self { - let mut visited = BitSet::new_empty(graph.num_nodes()); - visited.insert(start_node); - Self { graph, stack: vec![start_node], visited } + pub fn new(graph: &'graph G) -> Self { + Self { graph, stack: vec![], visited: BitSet::new_empty(graph.num_nodes()) } + } + + /// Version of `push_start_node` that is convenient for chained + /// use. + pub fn with_start_node(mut self, start_node: G::Node) -> Self { + self.push_start_node(start_node); + self + } + + /// Pushes another start node onto the stack. If the node + /// has not already been visited, then you will be able to + /// walk its successors (and so forth) after the current + /// contents of the stack are drained. If multiple start nodes + /// are added into the walk, then their mutual successors + /// will all be walked. You can use this method once the + /// iterator has been completely drained to add additional + /// start nodes. + pub fn push_start_node(&mut self, start_node: G::Node) { + if self.visited.insert(start_node) { + self.stack.push(start_node); + } + } + + /// Searches all nodes reachable from the current start nodes. + /// This is equivalent to just invoke `next` repeatedly until + /// you get a `None` result. + pub fn complete_search(&mut self) { + while let Some(_) = self.next() {} } + + /// Returns true if node has been visited thus far. + /// A node is considered "visited" once it is pushed + /// onto the internal stack; it may not yet have been yielded + /// from the iterator. This method is best used after + /// the iterator is completely drained. + pub fn visited(&self, node: G::Node) -> bool { + self.visited.contains(node) + } +} + +impl std::fmt::Debug for DepthFirstSearch<'_, G> +where + G: ?Sized + DirectedGraph + WithNumNodes + WithSuccessors, +{ + fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut f = fmt.debug_set(); + for n in self.visited.iter() { + f.entry(&n); + } + f.finish() } } diff --git a/compiler/rustc_data_structures/src/graph/iterate/tests.rs b/compiler/rustc_data_structures/src/graph/iterate/tests.rs index 3ee03d8b29214..fbdcb01b5f453 100644 --- a/compiler/rustc_data_structures/src/graph/iterate/tests.rs +++ b/compiler/rustc_data_structures/src/graph/iterate/tests.rs @@ -25,6 +25,14 @@ fn is_cyclic() { fn dfs() { let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (2, 3), (3, 0)]); - let result: Vec = DepthFirstSearch::new(&graph, 0).collect(); + let result: Vec = DepthFirstSearch::new(&graph).with_start_node(0).collect(); assert_eq!(result, vec![0, 2, 3, 1]); } + +#[test] +fn dfs_debug() { + let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (2, 3), (3, 0)]); + let mut dfs = DepthFirstSearch::new(&graph).with_start_node(0); + while let Some(_) = dfs.next() {} + assert_eq!(format!("{{0, 1, 2, 3}}"), format!("{:?}", dfs)); +} diff --git a/compiler/rustc_data_structures/src/graph/mod.rs b/compiler/rustc_data_structures/src/graph/mod.rs index e0903e4324124..684c1abe45cb9 100644 --- a/compiler/rustc_data_structures/src/graph/mod.rs +++ b/compiler/rustc_data_structures/src/graph/mod.rs @@ -32,7 +32,7 @@ where where Self: WithNumNodes, { - iterate::DepthFirstSearch::new(self, from) + iterate::DepthFirstSearch::new(self).with_start_node(from) } } diff --git a/compiler/rustc_typeck/src/check/fallback.rs b/compiler/rustc_typeck/src/check/fallback.rs index 834827633aad5..1bb11334a5757 100644 --- a/compiler/rustc_typeck/src/check/fallback.rs +++ b/compiler/rustc_typeck/src/check/fallback.rs @@ -1,6 +1,9 @@ use crate::check::FnCtxt; use rustc_data_structures::{ - fx::FxHashMap, graph::vec_graph::VecGraph, graph::WithSuccessors, stable_set::FxHashSet, + fx::FxHashMap, + graph::WithSuccessors, + graph::{iterate::DepthFirstSearch, vec_graph::VecGraph}, + stable_set::FxHashSet, }; use rustc_middle::ty::{self, Ty}; @@ -275,7 +278,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> { // 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 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 { @@ -288,16 +291,21 @@ impl<'tcx> FnCtxt<'_, 'tcx> { ); 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::>() ); - roots_reachable_from_diverging.extend(coercion_graph.depth_first_search(root_vid)); + + // 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, @@ -307,13 +315,14 @@ impl<'tcx> FnCtxt<'_, 'tcx> { // 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(); + 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.contains(&root_vid) { + if roots_reachable_from_diverging.visited(root_vid) { continue; } - roots_reachable_from_non_diverging.extend(coercion_graph.depth_first_search(root_vid)); + 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={:?}", @@ -329,7 +338,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> { 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)); + .any(|n| roots_reachable_from_non_diverging.visited(n)); if can_reach_non_diverging { debug!("fallback to (): {:?}", diverging_vid); diverging_fallback.insert(diverging_ty, self.tcx.types.unit);