From 89271352741e54c08c89fe19b6832666a29a932d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 24 Nov 2022 18:45:02 +0000 Subject: [PATCH 1/3] Assert that we don't capture escaping bound vars in Fn trait selection --- .../src/traits/select/confirmation.rs | 13 ++++++++----- .../rustc_trait_selection/src/traits/select/mod.rs | 14 +++++++++----- compiler/rustc_trait_selection/src/traits/util.rs | 6 +++--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index c0edbebed54..8f473ebb452 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -693,16 +693,19 @@ fn confirm_generator_candidate( let gen_sig = substs.as_generator().poly_sig(); - // (1) Feels icky to skip the binder here, but OTOH we know - // that the self-type is an generator type and hence is + // NOTE: The self-type is a generator type and hence is // in fact unparameterized (or at least does not reference any - // regions bound in the obligation). Still probably some - // refactoring could make this nicer. + // regions bound in the obligation). + let self_ty = obligation + .predicate + .self_ty() + .no_bound_vars() + .expect("unboxed closure type should not capture bound vars from the predicate"); let trait_ref = super::util::generator_trait_ref_and_outputs( self.tcx(), obligation.predicate.def_id(), - obligation.predicate.skip_binder().self_ty(), // (1) + self_ty, gen_sig, ) .map_bound(|(trait_ref, ..)| trait_ref); diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 9fe13fe296a..2a1494e8952 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2271,15 +2271,19 @@ fn closure_trait_ref_unnormalized( debug!(?closure_sig); - // (1) Feels icky to skip the binder here, but OTOH we know - // that the self-type is an unboxed closure type and hence is + // NOTE: The self-type is an unboxed closure type and hence is // in fact unparameterized (or at least does not reference any - // regions bound in the obligation). Still probably some - // refactoring could make this nicer. + // regions bound in the obligation). + let self_ty = obligation + .predicate + .self_ty() + .no_bound_vars() + .expect("unboxed closure type should not capture bound vars from the predicate"); + closure_trait_ref_and_return_type( self.tcx(), obligation.predicate.def_id(), - obligation.predicate.skip_binder().self_ty(), // (1) + self_ty, closure_sig, util::TupleArgumentsFlag::No, ) diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index 20d8a7a742c..a496cea0b00 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -298,11 +298,11 @@ pub fn closure_trait_ref_and_return_type<'tcx>( sig: ty::PolyFnSig<'tcx>, tuple_arguments: TupleArgumentsFlag, ) -> ty::Binder<'tcx, (ty::TraitRef<'tcx>, Ty<'tcx>)> { + assert!(!self_ty.has_escaping_bound_vars()); let arguments_tuple = match tuple_arguments { TupleArgumentsFlag::No => sig.skip_binder().inputs()[0], TupleArgumentsFlag::Yes => tcx.intern_tup(sig.skip_binder().inputs()), }; - debug_assert!(!self_ty.has_escaping_bound_vars()); let trait_ref = tcx.mk_trait_ref(fn_trait_def_id, [self_ty, arguments_tuple]); sig.map_bound(|sig| (trait_ref, sig.output())) } @@ -313,7 +313,7 @@ pub fn generator_trait_ref_and_outputs<'tcx>( self_ty: Ty<'tcx>, sig: ty::PolyGenSig<'tcx>, ) -> ty::Binder<'tcx, (ty::TraitRef<'tcx>, Ty<'tcx>, Ty<'tcx>)> { - debug_assert!(!self_ty.has_escaping_bound_vars()); + assert!(!self_ty.has_escaping_bound_vars()); let trait_ref = tcx.mk_trait_ref(fn_trait_def_id, [self_ty, sig.skip_binder().resume_ty]); sig.map_bound(|sig| (trait_ref, sig.yield_ty, sig.return_ty)) } @@ -324,7 +324,7 @@ pub fn future_trait_ref_and_outputs<'tcx>( self_ty: Ty<'tcx>, sig: ty::PolyGenSig<'tcx>, ) -> ty::Binder<'tcx, (ty::TraitRef<'tcx>, Ty<'tcx>)> { - debug_assert!(!self_ty.has_escaping_bound_vars()); + assert!(!self_ty.has_escaping_bound_vars()); let trait_ref = tcx.mk_trait_ref(fn_trait_def_id, [self_ty]); sig.map_bound(|sig| (trait_ref, sig.return_ty)) } From c7330c9fe82b698831105fd31f611fb46586fd3c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 24 Nov 2022 19:37:03 +0000 Subject: [PATCH 2/3] Also check that fn pointer candidates don't have escaping bound vars --- .../rustc_trait_selection/src/traits/select/confirmation.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 8f473ebb452..db0f230cf8d 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -606,7 +606,10 @@ fn confirm_fn_pointer_candidate( debug!(?obligation, "confirm_fn_pointer_candidate"); // Okay to skip binder; it is reintroduced below. - let self_ty = self.infcx.shallow_resolve(obligation.self_ty().skip_binder()); + let self_ty = self + .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()); let trait_ref = closure_trait_ref_and_return_type( self.tcx(), From d945967779f8f4c72cb50e0793b039e36ecb9213 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 24 Nov 2022 19:55:47 +0000 Subject: [PATCH 3/3] Remove comment, simplify since we asserted fn ptr Self type has no bound vars --- .../src/traits/select/confirmation.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index db0f230cf8d..e46441001b5 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -605,7 +605,6 @@ fn confirm_fn_pointer_candidate( { debug!(?obligation, "confirm_fn_pointer_candidate"); - // Okay to skip binder; it is reintroduced below. let self_ty = self .infcx .shallow_resolve(obligation.self_ty().no_bound_vars()) @@ -624,15 +623,7 @@ fn confirm_fn_pointer_candidate( // Confirm the `type Output: Sized;` bound that is present on `FnOnce` let cause = obligation.derived_cause(BuiltinDerivedObligation); - // The binder on the Fn obligation is "less" important than the one on - // the signature, as evidenced by how we treat it during projection. - // The safe thing to do here is to liberate it, though, which should - // have no worse effect than skipping the binder here. - let liberated_fn_ty = - self.infcx.replace_bound_vars_with_placeholders(obligation.predicate.rebind(self_ty)); - let output_ty = self - .infcx - .replace_bound_vars_with_placeholders(liberated_fn_ty.fn_sig(self.tcx()).output()); + let output_ty = self.infcx.replace_bound_vars_with_placeholders(sig.output()); let output_ty = normalize_with_depth_to( self, obligation.param_env,