From ebe21ac23a8a259079c9c59669c1d7da1fa88d0c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 16 Jul 2021 17:34:23 +0000 Subject: [PATCH] Infer all inference variables via InferCx The previous algorithm was correct for the example given in its documentation, but when the TAIT was declared as a free item instead of an associated item, the generic parameters were the wrong ones. --- compiler/rustc_middle/src/ty/mod.rs | 2 +- compiler/rustc_typeck/src/check/mod.rs | 117 +----------------- compiler/rustc_typeck/src/check/writeback.rs | 52 ++++---- .../ui/type-alias-impl-trait/issue-60371.rs | 2 +- .../type-alias-impl-trait/issue-60371.stderr | 7 +- 5 files changed, 36 insertions(+), 144 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 156e860e1a3fb..f1fa964628b33 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -856,7 +856,7 @@ impl<'tcx> InstantiatedPredicates<'tcx> { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable, TypeFoldable)] pub struct OpaqueTypeKey<'tcx> { pub def_id: DefId, pub substs: SubstsRef<'tcx>, diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index ff7d291d3c909..34d0908bcc74e 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -112,11 +112,9 @@ use rustc_hir::{HirIdMap, ImplicitSelfKind, Node}; use rustc_index::bit_set::BitSet; use rustc_index::vec::Idx; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; -use rustc_middle::ty::fold::{TypeFoldable, TypeFolder}; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::subst::{InternalSubsts, Subst, SubstsRef}; -use rustc_middle::ty::{self, RegionKind, Ty, TyCtxt, UserType}; +use rustc_middle::ty::{self, Ty, TyCtxt, UserType}; use rustc_session::config; use rustc_session::parse::feature_err; use rustc_session::Session; @@ -321,117 +319,6 @@ fn used_trait_imports(tcx: TyCtxt<'_>, def_id: LocalDefId) -> &FxHashSet Self::MyItem -/// } -/// impl MyTrait for T where T: Iterator { -/// type MyItem = impl Iterator; -/// fn use_it(self) -> Self::MyItem { -/// self -/// } -/// } -/// ``` -/// -/// When we normalize the signature of `use_it` from the impl block, -/// we will normalize `Self::MyItem` to the opaque type `impl Iterator` -/// However, this projection result may contain inference variables, due -/// to the way that projection works. We didn't have any inference variables -/// in the signature to begin with - leaving them in will cause us to incorrectly -/// conclude that we don't have a defining use of `MyItem`. By mapping inference -/// variables back to the actual generic parameters, we will correctly see that -/// we have a defining use of `MyItem` -fn fixup_opaque_types<'tcx, T>(tcx: TyCtxt<'tcx>, val: T) -> T -where - T: TypeFoldable<'tcx>, -{ - struct FixupFolder<'tcx> { - tcx: TyCtxt<'tcx>, - } - - impl<'tcx> TypeFolder<'tcx> for FixupFolder<'tcx> { - fn tcx<'a>(&'a self) -> TyCtxt<'tcx> { - self.tcx - } - - fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { - match *ty.kind() { - ty::Opaque(def_id, substs) => { - debug!("fixup_opaque_types: found type {:?}", ty); - // Here, we replace any inference variables that occur within - // the substs of an opaque type. By definition, any type occurring - // in the substs has a corresponding generic parameter, which is what - // we replace it with. - // This replacement is only run on the function signature, so any - // inference variables that we come across must be the rust of projection - // (there's no other way for a user to get inference variables into - // a function signature). - if ty.needs_infer() { - let new_substs = InternalSubsts::for_item(self.tcx, def_id, |param, _| { - let old_param = substs[param.index as usize]; - match old_param.unpack() { - GenericArgKind::Type(old_ty) => { - if let ty::Infer(_) = old_ty.kind() { - // Replace inference type with a generic parameter - self.tcx.mk_param_from_def(param) - } else { - old_param.fold_with(self) - } - } - GenericArgKind::Const(old_const) => { - if let ty::ConstKind::Infer(_) = old_const.val { - // This should never happen - we currently do not support - // 'const projections', e.g.: - // `impl MyTrait for T where ::MyConst == 25` - // which should be the only way for us to end up with a const inference - // variable after projection. If Rust ever gains support for this kind - // of projection, this should *probably* be changed to - // `self.tcx.mk_param_from_def(param)` - bug!( - "Found infer const: `{:?}` in opaque type: {:?}", - old_const, - ty - ); - } else { - old_param.fold_with(self) - } - } - GenericArgKind::Lifetime(old_region) => { - if let RegionKind::ReVar(_) = old_region { - self.tcx.mk_param_from_def(param) - } else { - old_param.fold_with(self) - } - } - } - }); - let new_ty = self.tcx.mk_opaque(def_id, new_substs); - debug!("fixup_opaque_types: new type: {:?}", new_ty); - new_ty - } else { - ty - } - } - _ => ty.super_fold_with(self), - } - } - } - - debug!("fixup_opaque_types({:?})", val); - val.fold_with(&mut FixupFolder { tcx }) -} - fn typeck_const_arg<'tcx>( tcx: TyCtxt<'tcx>, (did, param_did): (LocalDefId, DefId), @@ -510,8 +397,6 @@ fn typeck_with_fallback<'tcx>( fn_sig, ); - let fn_sig = fixup_opaque_types(tcx, fn_sig); - let fcx = check_fn(&inh, param_env, fn_sig, decl, id, body, None).0; fcx } else { diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index 589570f1cb7ca..0aa059b7de80f 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -496,6 +496,8 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { debug_assert!(!instantiated_ty.has_escaping_bound_vars()); + let opaque_type_key = self.fcx.fully_resolve(opaque_type_key).unwrap(); + // Prevent: // * `fn foo() -> Foo` // * `fn foo() -> Foo` @@ -508,6 +510,8 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { // fn foo() -> Foo { .. } // ``` // figures out the concrete type with `U`, but the stored type is with `T`. + + // FIXME: why are we calling this here? This seems too early, and duplicated. let definition_ty = self.fcx.infer_opaque_definition_from_instantiation( opaque_type_key, instantiated_ty, @@ -529,33 +533,33 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { } } - if !opaque_type_key.substs.needs_infer() { - // We only want to add an entry into `concrete_opaque_types` - // if we actually found a defining usage of this opaque type. - // Otherwise, we do nothing - we'll either find a defining usage - // in some other location, or we'll end up emitting an error due - // to the lack of defining usage - if !skip_add { - let old_concrete_ty = self - .typeck_results - .concrete_opaque_types - .insert(opaque_type_key, definition_ty); - if let Some(old_concrete_ty) = old_concrete_ty { - if old_concrete_ty != definition_ty { - span_bug!( - span, - "`visit_opaque_types` tried to write different types for the same \ + if opaque_type_key.substs.needs_infer() { + span_bug!(span, "{:#?} has inference variables", opaque_type_key.substs) + } + + // We only want to add an entry into `concrete_opaque_types` + // if we actually found a defining usage of this opaque type. + // Otherwise, we do nothing - we'll either find a defining usage + // in some other location, or we'll end up emitting an error due + // to the lack of defining usage + if !skip_add { + let old_concrete_ty = self + .typeck_results + .concrete_opaque_types + .insert(opaque_type_key, definition_ty); + if let Some(old_concrete_ty) = old_concrete_ty { + if old_concrete_ty != definition_ty { + span_bug!( + span, + "`visit_opaque_types` tried to write different types for the same \ opaque type: {:?}, {:?}, {:?}, {:?}", - opaque_type_key.def_id, - definition_ty, - opaque_defn, - old_concrete_ty, - ); - } + opaque_type_key.def_id, + definition_ty, + opaque_defn, + old_concrete_ty, + ); } } - } else { - self.tcx().sess.delay_span_bug(span, "`opaque_defn` has inference variables"); } } } diff --git a/src/test/ui/type-alias-impl-trait/issue-60371.rs b/src/test/ui/type-alias-impl-trait/issue-60371.rs index 4ac7f9423ff41..b7c8a58a65629 100644 --- a/src/test/ui/type-alias-impl-trait/issue-60371.rs +++ b/src/test/ui/type-alias-impl-trait/issue-60371.rs @@ -9,7 +9,7 @@ trait Bug { impl Bug for &() { type Item = impl Bug; //~ ERROR `impl Trait` in type aliases is unstable //~^ ERROR the trait bound `(): Bug` is not satisfied - //~^^ ERROR could not find defining uses + //~^^ ERROR the trait bound `(): Bug` is not satisfied const FUN: fn() -> Self::Item = || (); //~^ ERROR type alias impl trait is not permitted here diff --git a/src/test/ui/type-alias-impl-trait/issue-60371.stderr b/src/test/ui/type-alias-impl-trait/issue-60371.stderr index 255d381bf0683..6857d5264b65e 100644 --- a/src/test/ui/type-alias-impl-trait/issue-60371.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-60371.stderr @@ -25,11 +25,14 @@ LL | type Item = impl Bug; = help: the following implementations were found: <&() as Bug> -error: could not find defining uses +error[E0277]: the trait bound `(): Bug` is not satisfied --> $DIR/issue-60371.rs:10:17 | LL | type Item = impl Bug; - | ^^^^^^^^ + | ^^^^^^^^ the trait `Bug` is not implemented for `()` + | + = help: the following implementations were found: + <&() as Bug> error: aborting due to 4 previous errors