From bc293ed53e9a747b12e03eee43df96d47e362bd2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 11 Dec 2022 19:46:58 +0000 Subject: [PATCH 1/3] bug! with a better error message for failing Instance::resolve --- compiler/rustc_codegen_cranelift/src/abi/mod.rs | 7 +++---- compiler/rustc_codegen_ssa/src/mir/block.rs | 11 +++++++---- compiler/rustc_middle/src/ty/instance.rs | 17 ++++++++++++++++- compiler/rustc_monomorphize/src/collector.rs | 14 ++++++++------ 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/abi/mod.rs b/compiler/rustc_codegen_cranelift/src/abi/mod.rs index 1e22537c2ba42..98b5fb1cce285 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -349,10 +349,9 @@ pub(crate) fn codegen_terminator_call<'tcx>( // Handle special calls like intrinsics and empty drop glue. let instance = if let ty::FnDef(def_id, substs) = *fn_ty.kind() { - let instance = ty::Instance::resolve(fx.tcx, ty::ParamEnv::reveal_all(), def_id, substs) - .unwrap() - .unwrap() - .polymorphize(fx.tcx); + let instance = + ty::Instance::expect_resolve(fx.tcx, ty::ParamEnv::reveal_all(), def_id, substs) + .polymorphize(fx.tcx); if fx.tcx.symbol_name(instance).name.starts_with("llvm.") { crate::intrinsics::codegen_llvm_intrinsic_call( diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index f3f5ddb52d6a4..831956cb2be38 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -751,10 +751,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let (instance, mut llfn) = match *callee.layout.ty.kind() { ty::FnDef(def_id, substs) => ( Some( - ty::Instance::resolve(bx.tcx(), ty::ParamEnv::reveal_all(), def_id, substs) - .unwrap() - .unwrap() - .polymorphize(bx.tcx()), + ty::Instance::expect_resolve( + bx.tcx(), + ty::ParamEnv::reveal_all(), + def_id, + substs, + ) + .polymorphize(bx.tcx()), ), None, ), diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 586460986dd73..35d369ffc891c 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -385,6 +385,21 @@ impl<'tcx> Instance<'tcx> { ) } + pub fn expect_resolve( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + def_id: DefId, + substs: SubstsRef<'tcx>, + ) -> Instance<'tcx> { + match ty::Instance::resolve(tcx, param_env, def_id, substs) { + Ok(Some(instance)) => instance, + _ => bug!( + "failed to resolve instance for {}", + tcx.def_path_str_with_substs(def_id, substs) + ), + } + } + // This should be kept up to date with `resolve`. pub fn resolve_opt_const_arg( tcx: TyCtxt<'tcx>, @@ -525,7 +540,7 @@ impl<'tcx> Instance<'tcx> { pub fn resolve_drop_in_place(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> ty::Instance<'tcx> { let def_id = tcx.require_lang_item(LangItem::DropInPlace, None); let substs = tcx.intern_substs(&[ty.into()]); - Instance::resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap().unwrap() + Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs) } #[instrument(level = "debug", skip(tcx), ret)] diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index cf7226a129ce7..10ea4d29cfe4e 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -931,10 +931,13 @@ fn visit_fn_use<'tcx>( ) { if let ty::FnDef(def_id, substs) = *ty.kind() { let instance = if is_direct_call { - ty::Instance::resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap().unwrap() + ty::Instance::expect_resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs) } else { - ty::Instance::resolve_for_fn_ptr(tcx, ty::ParamEnv::reveal_all(), def_id, substs) - .unwrap() + match ty::Instance::resolve_for_fn_ptr(tcx, ty::ParamEnv::reveal_all(), def_id, substs) + { + Some(instance) => instance, + _ => bug!("failed to resolve instance for {ty}"), + } }; visit_instance_use(tcx, instance, is_direct_call, source, output); } @@ -1369,9 +1372,8 @@ fn create_mono_items_for_default_impls<'tcx>( trait_ref.substs[param.index as usize] } }); - let instance = ty::Instance::resolve(tcx, param_env, method.def_id, substs) - .unwrap() - .unwrap(); + let instance = + ty::Instance::expect_resolve(tcx, param_env, method.def_id, substs); let mono_item = create_fn_mono_item(tcx, instance, DUMMY_SP); if mono_item.node.is_instantiable(tcx) && should_codegen_locally(tcx, &instance) From e7f9e1422ca1ae459628d8a79b11df8408f596a6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 1 Dec 2022 19:07:35 +0000 Subject: [PATCH 2/3] Add one to recursion depth in a few places --- .../rustc_trait_selection/src/traits/select/confirmation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index fda415155c469..b164d3d48391f 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -625,7 +625,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { self, obligation.param_env, cause.clone(), - obligation.recursion_depth, + obligation.recursion_depth + 1, output_ty, &mut nested, ); @@ -650,7 +650,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let trait_obligations = self.impl_or_trait_obligations( &obligation.cause, - obligation.recursion_depth, + obligation.recursion_depth + 1, obligation.param_env, trait_def_id, &substs, From 66fb34d20b844fbef44cec2e0c4f1ac255c67587 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 1 Dec 2022 19:30:05 +0000 Subject: [PATCH 3/3] Don't normalize in confirm_poly_trait_refs --- .../src/traits/select/confirmation.rs | 45 +++++++------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index b164d3d48391f..cf2933203c7e2 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -606,7 +606,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { .infcx .shallow_resolve(obligation.self_ty().no_bound_vars()) .expect("fn pointer should not capture bound vars from predicate"); - let sig = self_ty.fn_sig(self.tcx()); + // FnDef signatures are not necessarily normalized + let Normalized { value: sig, obligations: mut nested } = normalize_with_depth( + self, + obligation.param_env, + obligation.cause.clone(), + obligation.recursion_depth + 1, + self_ty.fn_sig(self.tcx()), + ); + let trait_ref = closure_trait_ref_and_return_type( self.tcx(), obligation.predicate.def_id(), @@ -616,22 +624,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ) .map_bound(|(trait_ref, _)| trait_ref); - let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?; + nested.extend(self.confirm_poly_trait_refs(obligation, trait_ref)?); // Confirm the `type Output: Sized;` bound that is present on `FnOnce` let cause = obligation.derived_cause(BuiltinDerivedObligation); - let output_ty = self.infcx.replace_bound_vars_with_placeholders(sig.output()); - let output_ty = normalize_with_depth_to( - self, - obligation.param_env, - cause.clone(), - obligation.recursion_depth + 1, - output_ty, - &mut nested, - ); - let tr = - ty::Binder::dummy(self.tcx().at(cause.span).mk_trait_ref(LangItem::Sized, [output_ty])); - nested.push(Obligation::new(self.infcx.tcx, cause, obligation.param_env, tr)); + let predicate = sig + .output() + .map_bound(|ty| self.tcx().at(cause.span).mk_trait_ref(LangItem::Sized, [ty])); + nested.push(Obligation::new(self.infcx.tcx, cause, obligation.param_env, predicate)); Ok(ImplSourceFnPointerData { fn_ty: self_ty, nested }) } @@ -799,25 +799,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { expected_trait_ref: ty::PolyTraitRef<'tcx>, ) -> Result>, SelectionError<'tcx>> { let obligation_trait_ref = obligation.predicate.to_poly_trait_ref(); - // Normalize the obligation and expected trait refs together, because why not - let Normalized { obligations: nested, value: (obligation_trait_ref, expected_trait_ref) } = - ensure_sufficient_stack(|| { - normalize_with_depth( - self, - obligation.param_env, - obligation.cause.clone(), - obligation.recursion_depth + 1, - (obligation_trait_ref, expected_trait_ref), - ) - }); - self.infcx .at(&obligation.cause, obligation.param_env) .sup(obligation_trait_ref, expected_trait_ref) - .map(|InferOk { mut obligations, .. }| { - obligations.extend(nested); - obligations - }) + .map(|InferOk { value: (), obligations }| obligations) .map_err(|e| OutputTypeParameterMismatch(expected_trait_ref, obligation_trait_ref, e)) }