From 45f441a7b41d09bb78b0ba3e260d2c868ef6add7 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 14 Nov 2022 17:42:46 +0100 Subject: [PATCH 1/4] nll: correctly deal with bivariance --- .../src/interpret/eval_context.rs | 6 +- .../src/transform/validate.rs | 85 +++++++++---------- .../rustc_infer/src/infer/nll_relate/mod.rs | 8 +- compiler/rustc_mir_transform/src/inline.rs | 12 +-- .../ui/mir/important-higher-ranked-regions.rs | 26 ++++++ 5 files changed, 82 insertions(+), 55 deletions(-) create mode 100644 src/test/ui/mir/important-higher-ranked-regions.rs diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index ab82268dde3ab..753c1e87d21f3 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -23,7 +23,7 @@ use super::{ MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, PointerArithmetic, Provenance, Scalar, StackPopJump, }; -use crate::transform::validate::equal_up_to_regions; +use crate::transform::validate; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// Stores the `Machine` instance. @@ -354,8 +354,8 @@ pub(super) fn mir_assign_valid_types<'tcx>( // Type-changing assignments can happen when subtyping is used. While // all normal lifetimes are erased, higher-ranked types with their // late-bound lifetimes are still around and can lead to type - // differences. So we compare ignoring lifetimes. - if equal_up_to_regions(tcx, param_env, src.ty, dest.ty) { + // differences. + if validate::is_subtype(tcx, param_env, src.ty, dest.ty) { // Make sure the layout is equal, too -- just to be safe. Miri really // needs layout equality. For performance reason we skip this check when // the types are equal. Equal types *can* have different layouts when diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 81b82a21fa1a7..69e457f0a1a86 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -2,7 +2,8 @@ use rustc_data_structures::fx::FxHashSet; use rustc_index::bit_set::BitSet; -use rustc_infer::infer::TyCtxtInferExt; +use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt}; +use rustc_infer::traits::ObligationCause; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::visit::NonUseContext::VarDebugInfo; use rustc_middle::mir::visit::{PlaceContext, Visitor}; @@ -12,12 +13,12 @@ use rustc_middle::mir::{ ProjectionElem, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK, }; -use rustc_middle::ty::fold::BottomUpFolder; -use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable, TypeVisitable}; +use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitable}; use rustc_mir_dataflow::impls::MaybeStorageLive; use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_target::abi::{Size, VariantIdx}; +use rustc_trait_selection::traits::ObligationCtxt; #[derive(Copy, Clone, Debug)] enum EdgeKind { @@ -70,13 +71,11 @@ impl<'tcx> MirPass<'tcx> for Validator { } } -/// Returns whether the two types are equal up to lifetimes. -/// All lifetimes, including higher-ranked ones, get ignored for this comparison. -/// (This is unlike the `erasing_regions` methods, which keep higher-ranked lifetimes for soundness reasons.) +/// Returns whether the two types are equal up to subtyping. /// -/// The point of this function is to approximate "equal up to subtyping". However, -/// the approximation is incorrect as variance is ignored. -pub fn equal_up_to_regions<'tcx>( +/// This is used in case we don't know the expected subtyping direction +/// and still want to check whether anything is broken. +pub fn is_equal_up_to_subtyping<'tcx>( tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, src: Ty<'tcx>, @@ -87,27 +86,40 @@ pub fn equal_up_to_regions<'tcx>( return true; } - // Normalize lifetimes away on both sides, then compare. - let normalize = |ty: Ty<'tcx>| { - tcx.try_normalize_erasing_regions(param_env, ty).unwrap_or(ty).fold_with( - &mut BottomUpFolder { - tcx, - // FIXME: We erase all late-bound lifetimes, but this is not fully correct. - // If you have a type like ` fn(&'a u32) as SomeTrait>::Assoc`, - // this is not necessarily equivalent to `::Assoc`, - // since one may have an `impl SomeTrait for fn(&32)` and - // `impl SomeTrait for fn(&'static u32)` at the same time which - // specify distinct values for Assoc. (See also #56105) - lt_op: |_| tcx.lifetimes.re_erased, - // Leave consts and types unchanged. - ct_op: |ct| ct, - ty_op: |ty| ty, - }, - ) - }; - tcx.infer_ctxt().build().can_eq(param_env, normalize(src), normalize(dest)).is_ok() + // Check for subtyping in either direction. + is_subtype(tcx, param_env, src, dest) || is_subtype(tcx, param_env, dest, src) } +pub fn is_subtype<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + src: Ty<'tcx>, + dest: Ty<'tcx>, +) -> bool { + if src == dest { + return true; + } + + let mut builder = + tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble); + let infcx = builder.build(); + let ocx = ObligationCtxt::new(&infcx); + let cause = ObligationCause::dummy(); + let src = ocx.normalize(cause.clone(), param_env, src); + let dest = ocx.normalize(cause.clone(), param_env, dest); + let Ok(infer_ok) = infcx.at(&cause, param_env).sub(src, dest) else { + return false; + }; + let () = ocx.register_infer_ok_obligations(infer_ok); + let errors = ocx.select_all_or_error(); + // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing` + // we would get unification errors because we're unable to look into opaque types, + // even if they're constrained in our current function. + // + // It seems very unlikely that this hides any bugs. + let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types(); + errors.is_empty() +} struct TypeChecker<'a, 'tcx> { when: &'a str, body: &'a Body<'tcx>, @@ -183,22 +195,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return true; } - // Normalize projections and things like that. - // Type-changing assignments can happen when subtyping is used. While - // all normal lifetimes are erased, higher-ranked types with their - // late-bound lifetimes are still around and can lead to type - // differences. So we compare ignoring lifetimes. - - // First, try with reveal_all. This might not work in some cases, as the predicates - // can be cleared in reveal_all mode. We try the reveal first anyways as it is used - // by some other passes like inlining as well. - let param_env = self.param_env.with_reveal_all_normalized(self.tcx); - if equal_up_to_regions(self.tcx, param_env, src, dest) { - return true; - } - - // If this fails, we can try it without the reveal. - equal_up_to_regions(self.tcx, self.param_env, src, dest) + is_subtype(self.tcx, self.param_env, src, dest) } } diff --git a/compiler/rustc_infer/src/infer/nll_relate/mod.rs b/compiler/rustc_infer/src/infer/nll_relate/mod.rs index 600f94f095eac..74b2fb613b781 100644 --- a/compiler/rustc_infer/src/infer/nll_relate/mod.rs +++ b/compiler/rustc_infer/src/infer/nll_relate/mod.rs @@ -556,8 +556,12 @@ where self.ambient_variance_info = self.ambient_variance_info.xform(info); debug!(?self.ambient_variance); - - let r = self.relate(a, b)?; + // In a bivariant context this always succeeds. + let r = if self.ambient_variance == ty::Variance::Bivariant { + a + } else { + self.relate(a, b)? + }; self.ambient_variance = old_ambient_variance; diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 780b91d9215d5..2084492cc6380 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -1,7 +1,6 @@ //! Inlining pass for MIR functions use crate::deref_separator::deref_finder; use rustc_attr::InlineAttr; -use rustc_const_eval::transform::validate::equal_up_to_regions; use rustc_index::bit_set::BitSet; use rustc_index::vec::Idx; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; @@ -14,7 +13,8 @@ use rustc_span::{hygiene::ExpnKind, ExpnData, LocalExpnId, Span}; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; -use super::simplify::{remove_dead_blocks, CfgSimplifier}; +use crate::simplify::{remove_dead_blocks, CfgSimplifier}; +use crate::validate; use crate::MirPass; use std::iter; use std::ops::{Range, RangeFrom}; @@ -180,7 +180,7 @@ impl<'tcx> Inliner<'tcx> { let TerminatorKind::Call { args, destination, .. } = &terminator.kind else { bug!() }; let destination_ty = destination.ty(&caller_body.local_decls, self.tcx).ty; let output_type = callee_body.return_ty(); - if !equal_up_to_regions(self.tcx, self.param_env, output_type, destination_ty) { + if !validate::is_subtype(self.tcx, self.param_env, output_type, destination_ty) { trace!(?output_type, ?destination_ty); return Err("failed to normalize return type"); } @@ -200,7 +200,7 @@ impl<'tcx> Inliner<'tcx> { arg_tuple_tys.iter().zip(callee_body.args_iter().skip(skipped_args)) { let input_type = callee_body.local_decls[input].ty; - if !equal_up_to_regions(self.tcx, self.param_env, arg_ty, input_type) { + if !validate::is_subtype(self.tcx, self.param_env, input_type, arg_ty) { trace!(?arg_ty, ?input_type); return Err("failed to normalize tuple argument type"); } @@ -209,7 +209,7 @@ impl<'tcx> Inliner<'tcx> { for (arg, input) in args.iter().zip(callee_body.args_iter()) { let input_type = callee_body.local_decls[input].ty; let arg_ty = arg.ty(&caller_body.local_decls, self.tcx); - if !equal_up_to_regions(self.tcx, self.param_env, arg_ty, input_type) { + if !validate::is_subtype(self.tcx, self.param_env, input_type, arg_ty) { trace!(?arg_ty, ?input_type); return Err("failed to normalize argument type"); } @@ -847,7 +847,7 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { let parent = Place { local, projection: self.tcx.intern_place_elems(proj_base) }; let parent_ty = parent.ty(&self.callee_body.local_decls, self.tcx); let check_equal = |this: &mut Self, f_ty| { - if !equal_up_to_regions(this.tcx, this.param_env, ty, f_ty) { + if !validate::is_equal_up_to_subtyping(this.tcx, this.param_env, ty, f_ty) { trace!(?ty, ?f_ty); this.validation = Err("failed to normalize projection type"); return; diff --git a/src/test/ui/mir/important-higher-ranked-regions.rs b/src/test/ui/mir/important-higher-ranked-regions.rs new file mode 100644 index 0000000000000..cadfb3b66f297 --- /dev/null +++ b/src/test/ui/mir/important-higher-ranked-regions.rs @@ -0,0 +1,26 @@ +// check-pass +// compile-flags: -Zvalidate-mir + +// This test checks that bivariant parameters are handled correctly +// in the mir. +#![allow(coherence_leak_check)] +trait Trait { + type Assoc; +} + +struct Foo(T) +where + T: Trait; + +impl Trait for for<'a> fn(&'a ()) { + type Assoc = u32; +} +impl Trait for fn(&'static ()) { + type Assoc = String; +} + +fn foo(x: Foo fn(&'a ()), u32>) -> Foo { + x +} + +fn main() {} From 6aa611a84c6f383e52ba4167dfda2910e122ef8f Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 15 Nov 2022 13:42:14 +0100 Subject: [PATCH 2/4] mv utility methods into separate module --- .../src/interpret/eval_context.rs | 4 +- .../src/transform/validate.rs | 54 +---------------- .../src/util/compare_types.rs | 60 +++++++++++++++++++ compiler/rustc_const_eval/src/util/mod.rs | 2 + .../rustc_infer/src/infer/nll_relate/mod.rs | 7 +-- compiler/rustc_mir_transform/src/inline.rs | 10 ++-- 6 files changed, 72 insertions(+), 65 deletions(-) create mode 100644 compiler/rustc_const_eval/src/util/compare_types.rs diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 753c1e87d21f3..b5df63790cb07 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -23,7 +23,7 @@ use super::{ MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, PointerArithmetic, Provenance, Scalar, StackPopJump, }; -use crate::transform::validate; +use crate::util; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// Stores the `Machine` instance. @@ -355,7 +355,7 @@ pub(super) fn mir_assign_valid_types<'tcx>( // all normal lifetimes are erased, higher-ranked types with their // late-bound lifetimes are still around and can lead to type // differences. - if validate::is_subtype(tcx, param_env, src.ty, dest.ty) { + if util::is_subtype(tcx, param_env, src.ty, dest.ty) { // Make sure the layout is equal, too -- just to be safe. Miri really // needs layout equality. For performance reason we skip this check when // the types are equal. Equal types *can* have different layouts when diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 69e457f0a1a86..860dee5898057 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -2,8 +2,6 @@ use rustc_data_structures::fx::FxHashSet; use rustc_index::bit_set::BitSet; -use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt}; -use rustc_infer::traits::ObligationCause; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::visit::NonUseContext::VarDebugInfo; use rustc_middle::mir::visit::{PlaceContext, Visitor}; @@ -18,7 +16,6 @@ use rustc_mir_dataflow::impls::MaybeStorageLive; use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_target::abi::{Size, VariantIdx}; -use rustc_trait_selection::traits::ObligationCtxt; #[derive(Copy, Clone, Debug)] enum EdgeKind { @@ -71,55 +68,6 @@ impl<'tcx> MirPass<'tcx> for Validator { } } -/// Returns whether the two types are equal up to subtyping. -/// -/// This is used in case we don't know the expected subtyping direction -/// and still want to check whether anything is broken. -pub fn is_equal_up_to_subtyping<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ParamEnv<'tcx>, - src: Ty<'tcx>, - dest: Ty<'tcx>, -) -> bool { - // Fast path. - if src == dest { - return true; - } - - // Check for subtyping in either direction. - is_subtype(tcx, param_env, src, dest) || is_subtype(tcx, param_env, dest, src) -} - -pub fn is_subtype<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ParamEnv<'tcx>, - src: Ty<'tcx>, - dest: Ty<'tcx>, -) -> bool { - if src == dest { - return true; - } - - let mut builder = - tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble); - let infcx = builder.build(); - let ocx = ObligationCtxt::new(&infcx); - let cause = ObligationCause::dummy(); - let src = ocx.normalize(cause.clone(), param_env, src); - let dest = ocx.normalize(cause.clone(), param_env, dest); - let Ok(infer_ok) = infcx.at(&cause, param_env).sub(src, dest) else { - return false; - }; - let () = ocx.register_infer_ok_obligations(infer_ok); - let errors = ocx.select_all_or_error(); - // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing` - // we would get unification errors because we're unable to look into opaque types, - // even if they're constrained in our current function. - // - // It seems very unlikely that this hides any bugs. - let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types(); - errors.is_empty() -} struct TypeChecker<'a, 'tcx> { when: &'a str, body: &'a Body<'tcx>, @@ -195,7 +143,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return true; } - is_subtype(self.tcx, self.param_env, src, dest) + crate::util::is_subtype(self.tcx, self.param_env, src, dest) } } diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs new file mode 100644 index 0000000000000..a1fce8cbd0f09 --- /dev/null +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -0,0 +1,60 @@ +//! Routines to check for relations between fully inferred types. +//! +//! FIXME: Move this to a more general place. The utility of this extends to +//! other areas of the compiler as well. + +use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt}; +use rustc_infer::traits::ObligationCause; +use rustc_middle::ty::{ParamEnv, Ty, TyCtxt}; +use rustc_trait_selection::traits::ObligationCtxt; + +/// Returns whether the two types are equal up to subtyping. +/// +/// This is used in case we don't know the expected subtyping direction +/// and still want to check whether anything is broken. +pub fn is_equal_up_to_subtyping<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + src: Ty<'tcx>, + dest: Ty<'tcx>, +) -> bool { + // Fast path. + if src == dest { + return true; + } + + // Check for subtyping in either direction. + is_subtype(tcx, param_env, src, dest) || is_subtype(tcx, param_env, dest, src) +} + +/// Returns whether `src` is a subtype of `dest`, i.e. `src <: dest`. +pub fn is_subtype<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + src: Ty<'tcx>, + dest: Ty<'tcx>, +) -> bool { + if src == dest { + return true; + } + + let mut builder = + tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble); + let infcx = builder.build(); + let ocx = ObligationCtxt::new(&infcx); + let cause = ObligationCause::dummy(); + let src = ocx.normalize(cause.clone(), param_env, src); + let dest = ocx.normalize(cause.clone(), param_env, dest); + let Ok(infer_ok) = infcx.at(&cause, param_env).sub(src, dest) else { + return false; + }; + let () = ocx.register_infer_ok_obligations(infer_ok); + let errors = ocx.select_all_or_error(); + // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing` + // we would get unification errors because we're unable to look into opaque types, + // even if they're constrained in our current function. + // + // It seems very unlikely that this hides any bugs. + let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types(); + errors.is_empty() +} diff --git a/compiler/rustc_const_eval/src/util/mod.rs b/compiler/rustc_const_eval/src/util/mod.rs index 4d0f81a406008..76ea5a24e69ed 100644 --- a/compiler/rustc_const_eval/src/util/mod.rs +++ b/compiler/rustc_const_eval/src/util/mod.rs @@ -2,6 +2,7 @@ pub mod aggregate; mod alignment; mod call_kind; pub mod collect_writes; +mod compare_types; mod find_self_call; mod might_permit_raw_init; mod type_name; @@ -9,6 +10,7 @@ mod type_name; pub use self::aggregate::expand_aggregate; pub use self::alignment::is_disaligned; pub use self::call_kind::{call_kind, CallDesugaringKind, CallKind}; +pub use self::compare_types::{is_equal_up_to_subtyping, is_subtype}; pub use self::find_self_call::find_self_call; pub use self::might_permit_raw_init::might_permit_raw_init; pub use self::type_name::type_name; diff --git a/compiler/rustc_infer/src/infer/nll_relate/mod.rs b/compiler/rustc_infer/src/infer/nll_relate/mod.rs index 74b2fb613b781..167a82d4499a1 100644 --- a/compiler/rustc_infer/src/infer/nll_relate/mod.rs +++ b/compiler/rustc_infer/src/infer/nll_relate/mod.rs @@ -557,11 +557,8 @@ where debug!(?self.ambient_variance); // In a bivariant context this always succeeds. - let r = if self.ambient_variance == ty::Variance::Bivariant { - a - } else { - self.relate(a, b)? - }; + let r = + if self.ambient_variance == ty::Variance::Bivariant { a } else { self.relate(a, b)? }; self.ambient_variance = old_ambient_variance; diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 2084492cc6380..d7dd5fc852845 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -14,7 +14,7 @@ use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; use crate::simplify::{remove_dead_blocks, CfgSimplifier}; -use crate::validate; +use crate::util; use crate::MirPass; use std::iter; use std::ops::{Range, RangeFrom}; @@ -180,7 +180,7 @@ impl<'tcx> Inliner<'tcx> { let TerminatorKind::Call { args, destination, .. } = &terminator.kind else { bug!() }; let destination_ty = destination.ty(&caller_body.local_decls, self.tcx).ty; let output_type = callee_body.return_ty(); - if !validate::is_subtype(self.tcx, self.param_env, output_type, destination_ty) { + if !util::is_subtype(self.tcx, self.param_env, output_type, destination_ty) { trace!(?output_type, ?destination_ty); return Err("failed to normalize return type"); } @@ -200,7 +200,7 @@ impl<'tcx> Inliner<'tcx> { arg_tuple_tys.iter().zip(callee_body.args_iter().skip(skipped_args)) { let input_type = callee_body.local_decls[input].ty; - if !validate::is_subtype(self.tcx, self.param_env, input_type, arg_ty) { + if !util::is_subtype(self.tcx, self.param_env, input_type, arg_ty) { trace!(?arg_ty, ?input_type); return Err("failed to normalize tuple argument type"); } @@ -209,7 +209,7 @@ impl<'tcx> Inliner<'tcx> { for (arg, input) in args.iter().zip(callee_body.args_iter()) { let input_type = callee_body.local_decls[input].ty; let arg_ty = arg.ty(&caller_body.local_decls, self.tcx); - if !validate::is_subtype(self.tcx, self.param_env, input_type, arg_ty) { + if !util::is_subtype(self.tcx, self.param_env, input_type, arg_ty) { trace!(?arg_ty, ?input_type); return Err("failed to normalize argument type"); } @@ -847,7 +847,7 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { let parent = Place { local, projection: self.tcx.intern_place_elems(proj_base) }; let parent_ty = parent.ty(&self.callee_body.local_decls, self.tcx); let check_equal = |this: &mut Self, f_ty| { - if !validate::is_equal_up_to_subtyping(this.tcx, this.param_env, ty, f_ty) { + if !util::is_equal_up_to_subtyping(this.tcx, this.param_env, ty, f_ty) { trace!(?ty, ?f_ty); this.validation = Err("failed to normalize projection type"); return; From f5f67618e25be01d8580e3d49033e0f39a3e7781 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 15 Nov 2022 13:43:57 +0100 Subject: [PATCH 3/4] add comment about opaque types --- compiler/rustc_const_eval/src/util/compare_types.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs index a1fce8cbd0f09..8a317a78afbcd 100644 --- a/compiler/rustc_const_eval/src/util/compare_types.rs +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -28,6 +28,9 @@ pub fn is_equal_up_to_subtyping<'tcx>( } /// Returns whether `src` is a subtype of `dest`, i.e. `src <: dest`. +/// +/// This mostly ignores opaque types as it can be used in constraining contexts +/// while still computing the final underlying type. pub fn is_subtype<'tcx>( tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, From b2e6d08e3f88aee01d84412e0ea8369c48e0f419 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 15 Nov 2022 14:05:30 +0100 Subject: [PATCH 4/4] use `ocx` type relation routines --- .../src/util/compare_types.rs | 6 ++--- .../rustc_hir_analysis/src/check/check.rs | 4 +-- .../src/check/compare_method.rs | 12 +++------ .../src/traits/engine.rs | 26 ++++++++++++++----- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs index 8a317a78afbcd..a9cb191cc593f 100644 --- a/compiler/rustc_const_eval/src/util/compare_types.rs +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -48,10 +48,10 @@ pub fn is_subtype<'tcx>( let cause = ObligationCause::dummy(); let src = ocx.normalize(cause.clone(), param_env, src); let dest = ocx.normalize(cause.clone(), param_env, dest); - let Ok(infer_ok) = infcx.at(&cause, param_env).sub(src, dest) else { - return false; + match ocx.sub(&cause, param_env, src, dest) { + Ok(()) => {} + Err(_) => return false, }; - let () = ocx.register_infer_ok_obligations(infer_ok); let errors = ocx.select_all_or_error(); // With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing` // we would get unification errors because we're unable to look into opaque types, diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 0ba5e61510125..4b74dab709592 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -451,8 +451,8 @@ fn check_opaque_meets_bounds<'tcx>( let misc_cause = traits::ObligationCause::misc(span, hir_id); - match infcx.at(&misc_cause, param_env).eq(opaque_ty, hidden_ty) { - Ok(infer_ok) => ocx.register_infer_ok_obligations(infer_ok), + match ocx.eq(&misc_cause, param_env, opaque_ty, hidden_ty) { + Ok(()) => {} Err(ty_err) => { tcx.sess.delay_span_bug( span, diff --git a/compiler/rustc_hir_analysis/src/check/compare_method.rs b/compiler/rustc_hir_analysis/src/check/compare_method.rs index 7c99896b45714..af303b71118ec 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_method.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_method.rs @@ -402,10 +402,8 @@ pub fn collect_trait_impl_trait_tys<'tcx>( unnormalized_trait_sig.inputs_and_output.iter().chain(trait_sig.inputs_and_output.iter()), ); - match infcx.at(&cause, param_env).eq(trait_return_ty, impl_return_ty) { - Ok(infer::InferOk { value: (), obligations }) => { - ocx.register_obligations(obligations); - } + match ocx.eq(&cause, param_env, trait_return_ty, impl_return_ty) { + Ok(()) => {} Err(terr) => { let mut diag = struct_span_err!( tcx.sess, @@ -442,10 +440,8 @@ pub fn collect_trait_impl_trait_tys<'tcx>( // the lifetimes of the return type, but do this after unifying just the // return types, since we want to avoid duplicating errors from // `compare_predicate_entailment`. - match infcx.at(&cause, param_env).eq(trait_fty, impl_fty) { - Ok(infer::InferOk { value: (), obligations }) => { - ocx.register_obligations(obligations); - } + match ocx.eq(&cause, param_env, trait_fty, impl_fty) { + Ok(()) => {} Err(terr) => { // This function gets called during `compare_predicate_entailment` when normalizing a // signature that contains RPITIT. When the method signatures don't match, we have to diff --git a/compiler/rustc_trait_selection/src/traits/engine.rs b/compiler/rustc_trait_selection/src/traits/engine.rs index ae29c9f561791..f8346e515d70c 100644 --- a/compiler/rustc_trait_selection/src/traits/engine.rs +++ b/compiler/rustc_trait_selection/src/traits/engine.rs @@ -125,6 +125,21 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> { .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) } + /// Checks whether `expected` is a subtype of `actual`: `expected <: actual`. + pub fn sub>( + &self, + cause: &ObligationCause<'tcx>, + param_env: ty::ParamEnv<'tcx>, + expected: T, + actual: T, + ) -> Result<(), TypeError<'tcx>> { + self.infcx + .at(cause, param_env) + .sup(expected, actual) + .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) + } + + /// Checks whether `expected` is a supertype of `actual`: `expected :> actual`. pub fn sup>( &self, cause: &ObligationCause<'tcx>, @@ -132,13 +147,10 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> { expected: T, actual: T, ) -> Result<(), TypeError<'tcx>> { - match self.infcx.at(cause, param_env).sup(expected, actual) { - Ok(InferOk { obligations, value: () }) => { - self.register_obligations(obligations); - Ok(()) - } - Err(e) => Err(e), - } + self.infcx + .at(cause, param_env) + .sup(expected, actual) + .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) } pub fn select_where_possible(&self) -> Vec> {