From 32f687854a8f4d9c42c6611b4af5199aaff99791 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 10 Feb 2023 10:26:26 +0000 Subject: [PATCH 1/4] Stop bubbling out hidden types from the eval obligation queries --- compiler/rustc_trait_selection/src/infer.rs | 1 + compiler/rustc_traits/src/evaluate_obligation.rs | 10 +++------- compiler/rustc_traits/src/type_op.rs | 15 +++++---------- .../issue-53398-cyclic-types.stderr | 5 +---- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_trait_selection/src/infer.rs b/compiler/rustc_trait_selection/src/infer.rs index c0bfe152a1e8c..cf54ab8122c44 100644 --- a/compiler/rustc_trait_selection/src/infer.rs +++ b/compiler/rustc_trait_selection/src/infer.rs @@ -46,6 +46,7 @@ pub trait InferCtxtExt<'tcx> { param_env: ty::ParamEnv<'tcx>, ) -> traits::EvaluationResult; } + impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { fn type_is_copy_modulo_regions( &self, diff --git a/compiler/rustc_traits/src/evaluate_obligation.rs b/compiler/rustc_traits/src/evaluate_obligation.rs index e94c8efe69a92..9c72890a70a22 100644 --- a/compiler/rustc_traits/src/evaluate_obligation.rs +++ b/compiler/rustc_traits/src/evaluate_obligation.rs @@ -1,4 +1,4 @@ -use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt}; +use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{ParamEnvAnd, TyCtxt}; use rustc_span::source_map::DUMMY_SP; @@ -16,12 +16,8 @@ fn evaluate_obligation<'tcx>( canonical_goal: CanonicalPredicateGoal<'tcx>, ) -> Result { debug!("evaluate_obligation(canonical_goal={:#?})", canonical_goal); - // HACK This bubble is required for this tests to pass: - // impl-trait/issue99642.rs - let (ref infcx, goal, _canonical_inference_vars) = tcx - .infer_ctxt() - .with_opaque_type_inference(DefiningAnchor::Bubble) - .build_with_canonical(DUMMY_SP, &canonical_goal); + let (ref infcx, goal, _canonical_inference_vars) = + tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &canonical_goal); debug!("evaluate_obligation: goal={:#?}", goal); let ParamEnvAnd { param_env, value: predicate } = goal; diff --git a/compiler/rustc_traits/src/type_op.rs b/compiler/rustc_traits/src/type_op.rs index d34fce64dd7dc..7d1fb4735de1e 100644 --- a/compiler/rustc_traits/src/type_op.rs +++ b/compiler/rustc_traits/src/type_op.rs @@ -1,6 +1,6 @@ use rustc_hir as hir; use rustc_infer::infer::canonical::{Canonical, QueryResponse}; -use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt}; +use rustc_infer::infer::TyCtxtInferExt; use rustc_infer::traits::ObligationCauseCode; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, FnSig, Lift, PolyFnSig, Ty, TyCtxt, TypeFoldable}; @@ -212,15 +212,10 @@ fn type_op_prove_predicate<'tcx>( tcx: TyCtxt<'tcx>, canonicalized: Canonical<'tcx, ParamEnvAnd<'tcx, ProvePredicate<'tcx>>>, ) -> Result<&'tcx Canonical<'tcx, QueryResponse<'tcx, ()>>, NoSolution> { - // HACK This bubble is required for this test to pass: - // impl-trait/issue-99642.rs - tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).enter_canonical_trait_query( - &canonicalized, - |ocx, key| { - type_op_prove_predicate_with_cause(ocx, key, ObligationCause::dummy()); - Ok(()) - }, - ) + tcx.infer_ctxt().enter_canonical_trait_query(&canonicalized, |ocx, key| { + type_op_prove_predicate_with_cause(ocx, key, ObligationCause::dummy()); + Ok(()) + }) } /// The core of the `type_op_prove_predicate` query: for diagnostics purposes in NLL HRTB errors, diff --git a/tests/ui/type-alias-impl-trait/issue-53398-cyclic-types.stderr b/tests/ui/type-alias-impl-trait/issue-53398-cyclic-types.stderr index 0a34e8486a551..00c682b21939c 100644 --- a/tests/ui/type-alias-impl-trait/issue-53398-cyclic-types.stderr +++ b/tests/ui/type-alias-impl-trait/issue-53398-cyclic-types.stderr @@ -1,11 +1,8 @@ -error[E0275]: overflow evaluating the requirement `Foo: Sized` +error[E0275]: overflow evaluating the requirement ` Foo {foo} as FnOnce<()>>::Output == fn() -> Foo {foo}` --> $DIR/issue-53398-cyclic-types.rs:5:13 | LL | fn foo() -> Foo { | ^^^ - | - = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_53398_cyclic_types`) - = note: required because it appears within the type `fn() -> Foo {foo}` error: aborting due to previous error From 8c66d6d3e7dfb71fbb5172a66c294eedc3c00ba2 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 10 Feb 2023 12:49:56 +0000 Subject: [PATCH 2/4] Remove unused field --- .../src/infer/canonical/query_response.rs | 39 +++---------------- compiler/rustc_middle/src/infer/canonical.rs | 6 --- compiler/rustc_middle/src/traits/solve.rs | 32 +++++++-------- .../rustc_trait_selection/src/solve/mod.rs | 20 +++------- compiler/rustc_traits/src/chalk/mod.rs | 2 - 5 files changed, 24 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index b9cb9732ca3ed..ba89dd07f4890 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -91,7 +91,6 @@ impl<'tcx> InferCtxt<'tcx> { var_values: inference_vars, region_constraints: QueryRegionConstraints::default(), certainty: Certainty::Proven, // Ambiguities are OK! - opaque_types: vec![], value: answer, }) } @@ -140,28 +139,14 @@ impl<'tcx> InferCtxt<'tcx> { let certainty = if ambig_errors.is_empty() { Certainty::Proven } else { Certainty::Ambiguous }; - let opaque_types = self.take_opaque_types_for_query_response(); - Ok(QueryResponse { var_values: inference_vars, region_constraints, certainty, value: answer, - opaque_types, }) } - /// FIXME: This method should only be used for canonical queries and therefore be private. - /// - /// As the new solver does canonicalization slightly differently, this is also used there - /// for now. This should hopefully change fairly soon. - pub fn take_opaque_types_for_query_response(&self) -> Vec<(Ty<'tcx>, Ty<'tcx>)> { - std::mem::take(&mut self.inner.borrow_mut().opaque_type_storage.opaque_types) - .into_iter() - .map(|(k, v)| (self.tcx.mk_opaque(k.def_id.to_def_id(), k.substs), v.hidden_type.ty)) - .collect() - } - /// Given the (canonicalized) result to a canonical query, /// instantiates the result so it can be used, plugging in the /// values from the canonical query. (Note that the result may @@ -244,8 +229,8 @@ impl<'tcx> InferCtxt<'tcx> { where R: Debug + TypeFoldable<'tcx>, { - let InferOk { value: result_subst, mut obligations } = self - .query_response_substitution_guess(cause, param_env, original_values, query_response)?; + let InferOk { value: result_subst, mut obligations } = + self.query_response_substitution_guess(cause, original_values, query_response)?; // Compute `QueryOutlivesConstraint` values that unify each of // the original values `v_o` that was canonicalized into a @@ -363,12 +348,8 @@ impl<'tcx> InferCtxt<'tcx> { original_values, query_response, ); - let mut value = self.query_response_substitution_guess( - cause, - param_env, - original_values, - query_response, - )?; + let mut value = + self.query_response_substitution_guess(cause, original_values, query_response)?; value.obligations.extend( self.unify_query_response_substitution_guess( @@ -396,7 +377,6 @@ impl<'tcx> InferCtxt<'tcx> { fn query_response_substitution_guess( &self, cause: &ObligationCause<'tcx>, - param_env: ty::ParamEnv<'tcx>, original_values: &OriginalQueryValues<'tcx>, query_response: &Canonical<'tcx, QueryResponse<'tcx, R>>, ) -> InferResult<'tcx, CanonicalVarValues<'tcx>> @@ -496,16 +476,7 @@ impl<'tcx> InferCtxt<'tcx> { )), }; - let mut obligations = vec![]; - - // Carry all newly resolved opaque types to the caller's scope - for &(a, b) in &query_response.value.opaque_types { - let a = substitute_value(self.tcx, &result_subst, a); - let b = substitute_value(self.tcx, &result_subst, b); - obligations.extend(self.at(cause, param_env).eq(a, b)?.obligations); - } - - Ok(InferOk { value: result_subst, obligations }) + Ok(InferOk { value: result_subst, obligations: vec![] }) } /// Given a "guess" at the values for the canonical variables in diff --git a/compiler/rustc_middle/src/infer/canonical.rs b/compiler/rustc_middle/src/infer/canonical.rs index bb617e692ccea..ec01c6331fa3a 100644 --- a/compiler/rustc_middle/src/infer/canonical.rs +++ b/compiler/rustc_middle/src/infer/canonical.rs @@ -204,12 +204,6 @@ pub struct QueryResponse<'tcx, R> { pub var_values: CanonicalVarValues<'tcx>, pub region_constraints: QueryRegionConstraints<'tcx>, pub certainty: Certainty, - /// List of opaque types which we tried to compare to another type. - /// Inside the query we don't know yet whether the opaque type actually - /// should get its hidden type inferred. So we bubble the opaque type - /// and the type it was compared against upwards and let the query caller - /// handle it. - pub opaque_types: Vec<(Ty<'tcx>, Ty<'tcx>)>, pub value: R, } diff --git a/compiler/rustc_middle/src/traits/solve.rs b/compiler/rustc_middle/src/traits/solve.rs index bddf84880d297..69b7b0ec33161 100644 --- a/compiler/rustc_middle/src/traits/solve.rs +++ b/compiler/rustc_middle/src/traits/solve.rs @@ -4,7 +4,7 @@ use rustc_data_structures::intern::Interned; use crate::ty::{ ir::{self, TypeFoldable, TypeVisitable}, - FallibleTypeFolder, Ty, TyCtxt, TypeFolder, TypeVisitor, + FallibleTypeFolder, TyCtxt, TypeFolder, TypeVisitor, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -19,32 +19,27 @@ impl<'tcx> std::ops::Deref for ExternalConstraints<'tcx> { } /// Additional constraints returned on success. -#[derive(Debug, PartialEq, Eq, Clone, Hash, Default)] +#[derive(Debug, PartialEq, Eq, Clone, Hash)] pub struct ExternalConstraintsData<'tcx> { // FIXME: implement this. - pub regions: (), - pub opaque_types: Vec<(Ty<'tcx>, Ty<'tcx>)>, + pub regions: &'tcx (), +} + +impl<'tcx> Default for ExternalConstraintsData<'tcx> { + fn default() -> Self { + Self { regions: &() } + } } impl<'tcx> TypeFoldable> for ExternalConstraints<'tcx> { fn try_fold_with>(self, folder: &mut F) -> Result { - Ok(ir::FallibleTypeFolder::interner(folder).intern_external_constraints( - ExternalConstraintsData { - regions: (), - opaque_types: self - .opaque_types - .iter() - .map(|opaque| opaque.try_fold_with(folder)) - .collect::>()?, - }, - )) + Ok(ir::FallibleTypeFolder::interner(folder) + .intern_external_constraints(ExternalConstraintsData { regions: &() })) } fn fold_with>(self, folder: &mut F) -> Self { - ir::TypeFolder::interner(folder).intern_external_constraints(ExternalConstraintsData { - regions: (), - opaque_types: self.opaque_types.iter().map(|opaque| opaque.fold_with(folder)).collect(), - }) + ir::TypeFolder::interner(folder) + .intern_external_constraints(ExternalConstraintsData { regions: &() }) } } @@ -54,7 +49,6 @@ impl<'tcx> TypeVisitable> for ExternalConstraints<'tcx> { visitor: &mut V, ) -> std::ops::ControlFlow { self.regions.visit_with(visitor)?; - self.opaque_types.visit_with(visitor)?; ControlFlow::Continue(()) } } diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index 6890811fd046e..ffac172251849 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -92,10 +92,9 @@ impl<'tcx> CanonicalResponseExt for Canonical<'tcx, Response<'tcx>> { fn has_no_inference_or_external_constraints(&self) -> bool { // so that we get a compile error when regions are supported // so this code can be checked for being correct - let _: () = self.value.external_constraints.regions; + let _: &() = self.value.external_constraints.regions; self.value.var_values.is_identity() - && self.value.external_constraints.opaque_types.is_empty() } } @@ -582,14 +581,11 @@ fn compute_external_query_constraints<'tcx>( infcx: &InferCtxt<'tcx>, ) -> Result, NoSolution> { let region_obligations = infcx.take_registered_region_obligations(); - let opaque_types = infcx.take_opaque_types_for_query_response(); - Ok(infcx.tcx.intern_external_constraints(ExternalConstraintsData { - // FIXME: Now that's definitely wrong :) - // - // Should also do the leak check here I think - regions: drop(region_obligations), - opaque_types, - })) + // FIXME: Now that's definitely wrong :) + // + // Should also do the leak check here I think + drop(region_obligations); + Ok(infcx.tcx.intern_external_constraints(ExternalConstraintsData { regions: &() })) } fn instantiate_canonical_query_response<'tcx>( @@ -609,10 +605,6 @@ fn instantiate_canonical_query_response<'tcx>( Certainty::Yes => OldCertainty::Proven, Certainty::Maybe(_) => OldCertainty::Ambiguous, }, - // FIXME: This to_owned makes me sad, but we should eventually impl - // `instantiate_query_response_and_region_obligations` separately - // instead of piggybacking off of the old implementation. - opaque_types: resp.external_constraints.opaque_types.to_owned(), value: resp.certainty, }), ) else { bug!(); }; diff --git a/compiler/rustc_traits/src/chalk/mod.rs b/compiler/rustc_traits/src/chalk/mod.rs index f33f9edd6279e..82d28b9e0fd7b 100644 --- a/compiler/rustc_traits/src/chalk/mod.rs +++ b/compiler/rustc_traits/src/chalk/mod.rs @@ -131,7 +131,6 @@ pub(crate) fn evaluate_goal<'tcx>( var_values: CanonicalVarValues { var_values }, region_constraints: QueryRegionConstraints::default(), certainty: Certainty::Proven, - opaque_types: vec![], value: (), }, }; @@ -159,7 +158,6 @@ pub(crate) fn evaluate_goal<'tcx>( var_values: CanonicalVarValues::dummy(), region_constraints: QueryRegionConstraints::default(), certainty: Certainty::Ambiguous, - opaque_types: vec![], value: (), }, }; From 506f934768c7dc5d973b693d2eb56c5962c422f4 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 10 Feb 2023 14:31:56 +0000 Subject: [PATCH 3/4] Rename and clarify that Bubble is now just Ingore as it doesn't actually register hidden types anymore --- .../src/region_infer/opaque_types.rs | 15 ++++++------- .../src/util/compare_types.rs | 17 +++++++------- compiler/rustc_infer/src/infer/mod.rs | 17 +++++++++----- .../rustc_infer/src/infer/opaque_types.rs | 22 ++++++++++--------- .../src/traits/coherence.rs | 4 ++-- compiler/rustc_traits/src/codegen.rs | 12 ++++------ 6 files changed, 45 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index bb42301828d2f..4bb0a3046001f 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -272,10 +272,13 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { // This logic duplicates most of `check_opaque_meets_bounds`. // FIXME(oli-obk): Also do region checks here and then consider removing `check_opaque_meets_bounds` entirely. let param_env = self.tcx.param_env(def_id); - // HACK This bubble is required for this tests to pass: - // type-alias-impl-trait/issue-67844-nested-opaque.rs - let infcx = - self.tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).build(); + let infcx = self + .tcx + .infer_ctxt() + // HACK This bubble is required for this tests to pass: + // type-alias-impl-trait/issue-67844-nested-opaque.rs + .with_opaque_type_inference(DefiningAnchor::Ignore) + .build(); let ocx = ObligationCtxt::new(&infcx); // Require the hidden type to be well-formed with only the generics of the opaque type. // Defining use functions may have more bounds than the opaque type, which is ok, as long as the @@ -316,10 +319,6 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { // version. let errors = ocx.select_all_or_error(); - // This is still required for many(half of the tests in ui/type-alias-impl-trait) - // tests to pass - let _ = infcx.take_opaque_types(); - if errors.is_empty() { definition_ty } else { diff --git a/compiler/rustc_const_eval/src/util/compare_types.rs b/compiler/rustc_const_eval/src/util/compare_types.rs index f5f3d5de6b5a2..53ee17b0e698a 100644 --- a/compiler/rustc_const_eval/src/util/compare_types.rs +++ b/compiler/rustc_const_eval/src/util/compare_types.rs @@ -41,8 +41,15 @@ pub fn is_subtype<'tcx>( return true; } - let mut builder = - tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble); + let mut builder = tcx + .infer_ctxt() + .ignoring_regions() + // 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. + .with_opaque_type_inference(DefiningAnchor::Ignore); let infcx = builder.build(); let ocx = ObligationCtxt::new(&infcx); let cause = ObligationCause::dummy(); @@ -53,11 +60,5 @@ pub fn is_subtype<'tcx>( Err(_) => return false, }; 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.take_opaque_types(); errors.is_empty() } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index aa316b2dadb04..22227b0c964a9 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -244,11 +244,12 @@ impl<'tcx> InferCtxtInner<'tcx> { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum DefiningAnchor { - /// `DefId` of the item. + /// `DefId` of the item for which we check whether any opaque types may + /// have their hidden types registered. Bind(LocalDefId), - /// When opaque types are not resolved, we `Bubble` up, meaning - /// return the opaque/hidden type pair from query, for caller of query to handle it. - Bubble, + /// Used to treat all opaque types as in their defining scope. + /// Used for coherence, codegen and sanity checks. + Ignore, /// Used to catch type mismatch errors when handling opaque types. Error, } @@ -259,7 +260,7 @@ pub struct InferCtxt<'tcx> { /// The `DefId` of the item in whose context we are performing inference or typeck. /// It is used to check whether an opaque type use is a defining use. /// - /// If it is `DefiningAnchor::Bubble`, we can't resolve opaque types here and need to bubble up + /// If it is `DefiningAnchor::Ignore`, we can't resolve opaque types here and need to bubble up /// the obligation. This frequently happens for /// short lived InferCtxt within queries. The opaque type obligations are forwarded /// to the outside until the end up in an `InferCtxt` for typeck or borrowck. @@ -1340,7 +1341,11 @@ impl<'tcx> InferCtxt<'tcx> { #[instrument(level = "debug", skip(self), ret)] pub fn take_opaque_types(&self) -> opaque_types::OpaqueTypeMap<'tcx> { - debug_assert_ne!(self.defining_use_anchor, DefiningAnchor::Error); + debug_assert!( + matches!(self.defining_use_anchor, DefiningAnchor::Bind(_)), + "{:?}", + self.defining_use_anchor + ); std::mem::take(&mut self.inner.borrow_mut().opaque_type_storage.opaque_types) } diff --git a/compiler/rustc_infer/src/infer/opaque_types.rs b/compiler/rustc_infer/src/infer/opaque_types.rs index e783443502b86..ab2a8724ef87d 100644 --- a/compiler/rustc_infer/src/infer/opaque_types.rs +++ b/compiler/rustc_infer/src/infer/opaque_types.rs @@ -144,7 +144,7 @@ impl<'tcx> InferCtxt<'tcx> { // ``` self.opaque_type_origin(def_id)? } - DefiningAnchor::Bubble => self.opaque_type_origin_unchecked(def_id), + DefiningAnchor::Ignore => self.opaque_type_origin_unchecked(def_id), DefiningAnchor::Error => return None, }; if let ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, .. }) = *b.kind() { @@ -374,7 +374,7 @@ impl<'tcx> InferCtxt<'tcx> { pub fn opaque_type_origin(&self, def_id: LocalDefId) -> Option { let opaque_hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id); let parent_def_id = match self.defining_use_anchor { - DefiningAnchor::Bubble | DefiningAnchor::Error => return None, + DefiningAnchor::Ignore | DefiningAnchor::Error => return None, DefiningAnchor::Bind(bind) => bind, }; @@ -539,14 +539,16 @@ impl<'tcx> InferCtxt<'tcx> { let span = cause.span; let mut obligations = vec![]; - let prev = self.inner.borrow_mut().opaque_types().register( - OpaqueTypeKey { def_id, substs }, - OpaqueHiddenType { ty: hidden_ty, span }, - origin, - ); - if let Some(prev) = prev { - obligations = - self.at(&cause, param_env).eq_exp(a_is_expected, prev, hidden_ty)?.obligations; + if !matches!(self.defining_use_anchor, DefiningAnchor::Ignore) { + let prev = self.inner.borrow_mut().opaque_types().register( + OpaqueTypeKey { def_id, substs }, + OpaqueHiddenType { ty: hidden_ty, span }, + origin, + ); + if let Some(prev) = prev { + obligations = + self.at(&cause, param_env).eq_exp(a_is_expected, prev, hidden_ty)?.obligations; + } } let item_bounds = tcx.bound_explicit_item_bounds(def_id.to_def_id()); diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 599238e405dea..1f5cb6f51f95b 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -96,7 +96,7 @@ pub fn overlapping_impls( } let infcx = - tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).intercrate().build(); + tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Ignore).intercrate().build(); let selcx = &mut SelectionContext::new(&infcx); let overlaps = overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some(); @@ -108,7 +108,7 @@ pub fn overlapping_impls( // this time tracking intercrate ambiguity causes for better // diagnostics. (These take time and can lead to false errors.) let infcx = - tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).intercrate().build(); + tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Ignore).intercrate().build(); let selcx = &mut SelectionContext::new(&infcx); selcx.enable_tracking_intercrate_ambiguity_causes(); Some(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap()) diff --git a/compiler/rustc_traits/src/codegen.rs b/compiler/rustc_traits/src/codegen.rs index 6f81d343e0fd8..cc206a0438caa 100644 --- a/compiler/rustc_traits/src/codegen.rs +++ b/compiler/rustc_traits/src/codegen.rs @@ -32,10 +32,11 @@ pub fn codegen_select_candidate<'tcx>( let infcx = tcx .infer_ctxt() .ignoring_regions() - .with_opaque_type_inference(DefiningAnchor::Bubble) + // Opaque types may have gotten their hidden types constrained, but we can ignore them safely + // as they will get constrained elsewhere, too. + // (ouz-a) This is required for `type-alias-impl-trait/assoc-projection-ice.rs` to pass + .with_opaque_type_inference(DefiningAnchor::Ignore) .build(); - //~^ HACK `Bubble` is required for - // this test to pass: type-alias-impl-trait/assoc-projection-ice.rs let mut selcx = SelectionContext::new(&infcx); let obligation_cause = ObligationCause::dummy(); @@ -79,10 +80,5 @@ pub fn codegen_select_candidate<'tcx>( let impl_source = infcx.resolve_vars_if_possible(impl_source); let impl_source = infcx.tcx.erase_regions(impl_source); - // Opaque types may have gotten their hidden types constrained, but we can ignore them safely - // as they will get constrained elsewhere, too. - // (ouz-a) This is required for `type-alias-impl-trait/assoc-projection-ice.rs` to pass - let _ = infcx.take_opaque_types(); - Ok(&*tcx.arena.alloc(impl_source)) } From 018f210a875c59b4b70b124b70a92275aee54945 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 10 Feb 2023 16:42:53 +0000 Subject: [PATCH 4/4] Make hidden type registration opt-in, so that each site can be reviewed on its own and we have the right defaults for trait solvers --- .../src/region_infer/opaque_types.rs | 5 +++-- compiler/rustc_hir_typeck/src/closure.rs | 7 +++++-- compiler/rustc_hir_typeck/src/coercion.rs | 12 ++++++++---- compiler/rustc_hir_typeck/src/demand.rs | 4 ++-- compiler/rustc_hir_typeck/src/method/probe.rs | 3 --- compiler/rustc_infer/src/infer/at.rs | 2 +- compiler/rustc_infer/src/infer/mod.rs | 13 ++++++++----- compiler/rustc_infer/src/infer/opaque_types.rs | 7 +++++-- .../rustc_trait_selection/src/solve/infcx_ext.rs | 1 - .../rustc_trait_selection/src/traits/coherence.rs | 1 + .../rustc_trait_selection/src/traits/engine.rs | 4 ++++ .../rustc_trait_selection/src/traits/project.rs | 7 ++++++- .../src/traits/select/confirmation.rs | 2 ++ .../rustc_trait_selection/src/traits/select/mod.rs | 8 -------- .../ui/type-alias-impl-trait/match-unification.rs | 14 ++++++++++++++ 15 files changed, 59 insertions(+), 31 deletions(-) create mode 100644 tests/ui/type-alias-impl-trait/match-unification.rs diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 4bb0a3046001f..17c2aa1276948 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -275,8 +275,9 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { let infcx = self .tcx .infer_ctxt() - // HACK This bubble is required for this tests to pass: - // type-alias-impl-trait/issue-67844-nested-opaque.rs + // HACK This ignore is required for this tests to pass: + // nested-return-type2-tait2.rs + // nested-return-type2-tait3.rs .with_opaque_type_inference(DefiningAnchor::Ignore) .build(); let ocx = ObligationCtxt::new(&infcx); diff --git a/compiler/rustc_hir_typeck/src/closure.rs b/compiler/rustc_hir_typeck/src/closure.rs index cf296a7bf6530..c289eae8d2745 100644 --- a/compiler/rustc_hir_typeck/src/closure.rs +++ b/compiler/rustc_hir_typeck/src/closure.rs @@ -561,8 +561,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) { // Check that E' = S'. let cause = self.misc(hir_ty.span); - let InferOk { value: (), obligations } = - self.at(&cause, self.param_env).eq(*expected_ty, supplied_ty)?; + let InferOk { value: (), obligations } = self + .at(&cause, self.param_env) + .define_opaque_types(true) + .eq(*expected_ty, supplied_ty)?; all_obligations.extend(obligations); } @@ -574,6 +576,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let cause = &self.misc(decl.output.span()); let InferOk { value: (), obligations } = self .at(cause, self.param_env) + .define_opaque_types(true) .eq(expected_sigs.liberated_sig.output(), supplied_output_ty)?; all_obligations.extend(obligations); diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index ba503bf47e70e..a15fc9b57fbe6 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -143,11 +143,11 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> { debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub); self.commit_if_ok(|_| { + let at = self.at(&self.cause, self.fcx.param_env).define_opaque_types(true); if self.use_lub { - self.at(&self.cause, self.fcx.param_env).lub(b, a) + at.lub(b, a) } else { - self.at(&self.cause, self.fcx.param_env) - .sup(b, a) + at.sup(b, a) .map(|InferOk { value: (), obligations }| InferOk { value: a, obligations }) } }) @@ -174,7 +174,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { // Best-effort try to unify these types -- we're already on the error path, // so this will have the side-effect of making sure we have no ambiguities // due to `[type error]` and `_` not coercing together. - let _ = self.commit_if_ok(|_| self.at(&self.cause, self.param_env).eq(a, b)); + let _ = self.commit_if_ok(|_| { + self.at(&self.cause, self.param_env).define_opaque_types(true).eq(a, b) + }); return success(vec![], self.fcx.tcx.ty_error(), vec![]); } @@ -1484,6 +1486,8 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { // Another example is `break` with no argument expression. assert!(expression_ty.is_unit(), "if let hack without unit type"); fcx.at(cause, fcx.param_env) + // needed for tests/ui/type-alias-impl-trait/issue-65679-inst-opaque-ty-from-val-twice.rs + .define_opaque_types(true) .eq_exp(label_expression_as_expected, expression_ty, self.merged_ty()) .map(|infer_ok| { fcx.register_infer_ok_obligations(infer_ok); diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 879a64fc0fb9f..241547b70aa2f 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -113,7 +113,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, actual: Ty<'tcx>, ) -> Option> { - match self.at(cause, self.param_env).sup(expected, actual) { + match self.at(cause, self.param_env).define_opaque_types(true).sup(expected, actual) { Ok(InferOk { obligations, value: () }) => { self.register_predicates(obligations); None @@ -143,7 +143,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, actual: Ty<'tcx>, ) -> Option> { - match self.at(cause, self.param_env).eq(expected, actual) { + match self.at(cause, self.param_env).define_opaque_types(true).eq(expected, actual) { Ok(InferOk { obligations, value: () }) => { self.register_predicates(obligations); None diff --git a/compiler/rustc_hir_typeck/src/method/probe.rs b/compiler/rustc_hir_typeck/src/method/probe.rs index 88af554483b8e..e182de1af7c77 100644 --- a/compiler/rustc_hir_typeck/src/method/probe.rs +++ b/compiler/rustc_hir_typeck/src/method/probe.rs @@ -1488,7 +1488,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { TraitCandidate(trait_ref) => self.probe(|_| { let _ = self .at(&ObligationCause::dummy(), self.param_env) - .define_opaque_types(false) .sup(candidate.xform_self_ty, self_ty); match self.select_trait_candidate(trait_ref) { Ok(Some(traits::ImplSource::UserDefined(ref impl_data))) => { @@ -1518,7 +1517,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { // First check that the self type can be related. let sub_obligations = match self .at(&ObligationCause::dummy(), self.param_env) - .define_opaque_types(false) .sup(probe.xform_self_ty, self_ty) { Ok(InferOk { obligations, value: () }) => obligations, @@ -1735,7 +1733,6 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { if let ProbeResult::Match = result && self .at(&ObligationCause::dummy(), self.param_env) - .define_opaque_types(false) .sup(return_ty, xform_ret_ty) .is_err() { diff --git a/compiler/rustc_infer/src/infer/at.rs b/compiler/rustc_infer/src/infer/at.rs index d816a9ed3d7c1..c952ddc827a4a 100644 --- a/compiler/rustc_infer/src/infer/at.rs +++ b/compiler/rustc_infer/src/infer/at.rs @@ -55,7 +55,7 @@ impl<'tcx> InferCtxt<'tcx> { cause: &'a ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, ) -> At<'a, 'tcx> { - At { infcx: self, cause, param_env, define_opaque_types: true } + At { infcx: self, cause, param_env, define_opaque_types: false } } /// Forks the inference context, creating a new inference context with the same inference diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 22227b0c964a9..6c5b13377cfa0 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -260,12 +260,15 @@ pub struct InferCtxt<'tcx> { /// The `DefId` of the item in whose context we are performing inference or typeck. /// It is used to check whether an opaque type use is a defining use. /// - /// If it is `DefiningAnchor::Ignore`, we can't resolve opaque types here and need to bubble up - /// the obligation. This frequently happens for - /// short lived InferCtxt within queries. The opaque type obligations are forwarded - /// to the outside until the end up in an `InferCtxt` for typeck or borrowck. + /// If it is `DefiningAnchor::Ignore`, we can't resolve opaque types here and just + /// act as if we were in a defining scope. This is useful for codegen, as it reveals + /// opaque types, but some nested obligations may end up with opaque types and we don't + /// normalize obligations. It is also useful for coherence, as we want opaque types to + /// act as if they accept every type for which their bounds hold, no matter the defining scopes. + /// This is necessary as impls are accessible outside the defining scopes of types they get + /// implemented for. Finally, this is useful for sanity assertions in const eval. /// - /// It is default value is `DefiningAnchor::Error`, this way it is easier to catch errors that + /// Its default value is `DefiningAnchor::Error`, this way it is easier to catch errors that /// might come up during inference or typeck. pub defining_use_anchor: DefiningAnchor, diff --git a/compiler/rustc_infer/src/infer/opaque_types.rs b/compiler/rustc_infer/src/infer/opaque_types.rs index ab2a8724ef87d..de9335dc181d7 100644 --- a/compiler/rustc_infer/src/infer/opaque_types.rs +++ b/compiler/rustc_infer/src/infer/opaque_types.rs @@ -546,8 +546,11 @@ impl<'tcx> InferCtxt<'tcx> { origin, ); if let Some(prev) = prev { - obligations = - self.at(&cause, param_env).eq_exp(a_is_expected, prev, hidden_ty)?.obligations; + obligations = self + .at(&cause, param_env) + .define_opaque_types(true) + .eq_exp(a_is_expected, prev, hidden_ty)? + .obligations; } } diff --git a/compiler/rustc_trait_selection/src/solve/infcx_ext.rs b/compiler/rustc_trait_selection/src/solve/infcx_ext.rs index 36f987c9f9cb6..06570e1f4b41c 100644 --- a/compiler/rustc_trait_selection/src/solve/infcx_ext.rs +++ b/compiler/rustc_trait_selection/src/solve/infcx_ext.rs @@ -54,7 +54,6 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> { rhs: T, ) -> Result>>, NoSolution> { self.at(&ObligationCause::dummy(), param_env) - .define_opaque_types(false) .eq(lhs, rhs) .map(|InferOk { value: (), obligations }| { obligations.into_iter().map(|o| o.into()).collect() diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 1f5cb6f51f95b..0886c1ee73da6 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -217,6 +217,7 @@ fn equate_impl_headers<'cx, 'tcx>( selcx .infcx .at(&ObligationCause::dummy(), ty::ParamEnv::empty()) + .define_opaque_types(true) .eq_impl_headers(impl1_header, impl2_header) .map(|infer_ok| infer_ok.obligations) .ok() diff --git a/compiler/rustc_trait_selection/src/traits/engine.rs b/compiler/rustc_trait_selection/src/traits/engine.rs index a2ddd91546c18..4a7f08a35bc61 100644 --- a/compiler/rustc_trait_selection/src/traits/engine.rs +++ b/compiler/rustc_trait_selection/src/traits/engine.rs @@ -128,6 +128,7 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> { { self.infcx .at(cause, param_env) + .define_opaque_types(true) .eq_exp(a_is_expected, a, b) .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) } @@ -141,6 +142,7 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> { ) -> Result<(), TypeError<'tcx>> { self.infcx .at(cause, param_env) + .define_opaque_types(true) .eq(expected, actual) .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) } @@ -155,6 +157,7 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> { ) -> Result<(), TypeError<'tcx>> { self.infcx .at(cause, param_env) + .define_opaque_types(true) .sup(expected, actual) .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) } @@ -169,6 +172,7 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> { ) -> Result<(), TypeError<'tcx>> { self.infcx .at(cause, param_env) + .define_opaque_types(true) .sup(expected, actual) .map(|infer_ok| self.register_infer_ok_obligations(infer_ok)) } diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 1c66fb257ebb5..9ccdeca842ccd 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -286,7 +286,12 @@ fn project_and_unify_type<'cx, 'tcx>( ); obligations.extend(new); - match infcx.at(&obligation.cause, obligation.param_env).eq(normalized, actual) { + match infcx + .at(&obligation.cause, obligation.param_env) + // This is needed to support nested opaque types like `impl Fn() -> impl Trait` + .define_opaque_types(true) + .eq(normalized, actual) + { Ok(InferOk { obligations: inferred_obligations, value: () }) => { obligations.extend(inferred_obligations); ProjectAndUnifyResult::Holds(obligations) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index e4a832e472813..5c5b2d6dac3fb 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -821,6 +821,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { self.infcx .at(&obligation.cause, obligation.param_env) + // needed for tests/ui/type-alias-impl-trait/assoc-projection-ice.rs + .define_opaque_types(true) .sup(obligation_trait_ref, expected_trait_ref) .map(|InferOk { mut obligations, .. }| { obligations.extend(nested); diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 4b15dd408b370..79c188d7f815b 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1752,7 +1752,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { }); self.infcx .at(&obligation.cause, obligation.param_env) - .define_opaque_types(false) .sup(ty::Binder::dummy(placeholder_trait_ref), trait_bound) .map(|InferOk { obligations: _, value: () }| { // This method is called within a probe, so we can't have @@ -1815,7 +1814,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let is_match = self .infcx .at(&obligation.cause, obligation.param_env) - .define_opaque_types(false) .sup(obligation.predicate, infer_projection) .map_or(false, |InferOk { obligations, value: () }| { self.evaluate_predicates_recursively( @@ -2507,7 +2505,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let InferOk { obligations, .. } = self .infcx .at(&cause, obligation.param_env) - .define_opaque_types(false) .eq(placeholder_obligation_trait_ref, impl_trait_ref) .map_err(|e| { debug!("match_impl: failed eq_trait_refs due to `{}`", e.to_string(self.tcx())) @@ -2558,11 +2555,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ) -> Result>, ()> { self.infcx .at(&obligation.cause, obligation.param_env) - // We don't want predicates for opaque types to just match all other types, - // if there is an obligation on the opaque type, then that obligation must be met - // opaquely. Otherwise we'd match any obligation to the opaque type and then error - // out later. - .define_opaque_types(false) .sup(obligation.predicate.to_poly_trait_ref(), poly_trait_ref) .map(|InferOk { obligations, .. }| obligations) .map_err(|_| ()) diff --git a/tests/ui/type-alias-impl-trait/match-unification.rs b/tests/ui/type-alias-impl-trait/match-unification.rs new file mode 100644 index 0000000000000..f5c2abc0efa2e --- /dev/null +++ b/tests/ui/type-alias-impl-trait/match-unification.rs @@ -0,0 +1,14 @@ +use std::fmt::Debug; + +// check-pass + +fn bar() -> impl Debug {} + +fn baz(b: bool) -> Option { + match b { + true => baz(false), + false => Some(bar()), + } +} + +fn main() {}