Skip to content

Commit

Permalink
Auto merge of #119106 - lcnr:decrement-universes, r=BoxyUwU
Browse files Browse the repository at this point in the history
avoid generalization inside of aliases

The basic idea of this PR is that we don't generalize aliases when the instantiation could fail later on, either due to the *occurs check* or because of a universe error. We instead replace the whole alias with an inference variable and emit a nested `AliasRelate` goal. This `AliasRelate` then fully normalizes the alias before equating it with the inference variable, at which point the alias can be treated like any other rigid type.

We now treat aliases differently depending on whether they are *rigid* or not. To detect whether an alias is rigid we check whether `NormalizesTo` fails. While we already do so inside of `AliasRelate` anyways, also doing so when instantiating a query response would be both ugly/difficult and likely inefficient. To avoid that I change `instantiate_and_apply_query_response` to relate types completely structurally. This change generally removes a lot of annoying complexity, which is nice. It's implemented by adding a flag to `Equate` to change it to structurally handle aliases.

We currently always apply constraints from canonical queries right away. By providing all the necessary information to the canonical query, we can guarantee that instantiating the query response never fails, which further simplifies the implementation. This does add the invariant that *any information which could cause instantiating type variables to fail must also be available inside of the query*.

While it's acceptable for canonicalization to result in more ambiguity, we must not cause the solver to incompletely structurally relate aliases by erasing information. This means we have to be careful when merging universes during canonicalization. As we only generalize for type and const variables we have to make sure that anything nameable by such a type or const variable inside of the canonical query is also nameable outside of it. Because of this we both stop merging universes of existential variables when canonicalizing inputs, we put all uniquified regions into a higher universe which is not nameable by any type or const variable.

I will look into always replacing aliases with inference variables when generalizing in a later PR unless the alias references bound variables. This should both pretty much fix rust-lang/trait-system-refactor-initiative#4. This may allow us to merge the universes of existential variables again by changing generalize to not consider their universe when deciding whether to generalize aliases. This requires some additional non-trivial changes to alias-relate, so I am leaving that as future work.

Fixes rust-lang/trait-system-refactor-initiative#79. While it would be nice to decrement universe indices when existing a `forall`, that was surprisingly difficult and not necessary to fix this issue. I am really happy with the approach in this PR think it is the correct way forward to also fix the remaining cases of rust-lang/trait-system-refactor-initiative#8.
  • Loading branch information
bors committed Feb 26, 2024
2 parents ee933f6 + 3b3514a commit b8de591
Show file tree
Hide file tree
Showing 43 changed files with 586 additions and 366 deletions.
7 changes: 6 additions & 1 deletion compiler/rustc_borrowck/src/type_check/relate_tys.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::ErrorGuaranteed;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{NllRegionVariableOrigin, ObligationEmittingRelation};
use rustc_infer::infer::NllRegionVariableOrigin;
use rustc_infer::infer::{ObligationEmittingRelation, StructurallyRelateAliases};
use rustc_infer::traits::{Obligation, PredicateObligations};
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::traits::query::NoSolution;
Expand Down Expand Up @@ -548,6 +549,10 @@ impl<'bccx, 'tcx> ObligationEmittingRelation<'tcx> for NllTypeRelating<'_, 'bccx
self.locations.span(self.type_checker.body)
}

fn structurally_relate_aliases(&self) -> StructurallyRelateAliases {
StructurallyRelateAliases::No
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.type_checker.param_env
}
Expand Down
26 changes: 15 additions & 11 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ use rustc_hir::Expr;
use rustc_hir_analysis::astconv::AstConv;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{Coercion, DefineOpaqueTypes, InferOk, InferResult};
use rustc_infer::traits::TraitEngine;
use rustc_infer::traits::TraitEngineExt as _;
use rustc_infer::traits::{Obligation, PredicateObligation};
use rustc_middle::lint::in_external_macro;
use rustc_middle::traits::BuiltinImplSource;
Expand All @@ -61,6 +63,7 @@ use rustc_target::spec::abi::Abi;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::TraitEngineExt as _;
use rustc_trait_selection::traits::{
self, NormalizeExt, ObligationCause, ObligationCauseCode, ObligationCtxt,
};
Expand Down Expand Up @@ -157,17 +160,19 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// In the new solver, lazy norm may allow us to shallowly equate
// more types, but we emit possibly impossible-to-satisfy obligations.
// Filter these cases out to make sure our coercion is more accurate.
if self.next_trait_solver() {
if let Ok(res) = &res {
for obligation in &res.obligations {
if !self.predicate_may_hold(obligation) {
return Err(TypeError::Mismatch);
}
match res {
Ok(InferOk { value, obligations }) if self.next_trait_solver() => {
let mut fulfill_cx = <dyn TraitEngine<'tcx>>::new(self);
fulfill_cx.register_predicate_obligations(self, obligations);
let errs = fulfill_cx.select_where_possible(self);
if errs.is_empty() {
Ok(InferOk { value, obligations: fulfill_cx.pending_obligations() })
} else {
Err(TypeError::Mismatch)
}
}
res => res,
}

res
})
}

Expand Down Expand Up @@ -625,19 +630,18 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
let traits = [coerce_unsized_did, unsize_did];
while !queue.is_empty() {
let obligation = queue.remove(0);
debug!("coerce_unsized resolve step: {:?}", obligation);
let trait_pred = match obligation.predicate.kind().no_bound_vars() {
Some(ty::PredicateKind::Clause(ty::ClauseKind::Trait(trait_pred)))
if traits.contains(&trait_pred.def_id()) =>
{
trait_pred
self.resolve_vars_if_possible(trait_pred)
}
_ => {
coercion.obligations.push(obligation);
continue;
}
};
let trait_pred = self.resolve_vars_if_possible(trait_pred);
debug!("coerce_unsized resolve step: {:?}", trait_pred);
match selcx.select(&obligation.with(selcx.tcx(), trait_pred)) {
// Uncertain or unimplemented.
Ok(None) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1516,6 +1516,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// In case there is still ambiguity, the returned type may be an inference
/// variable. This is different from `structurally_resolve_type` which errors
/// in this case.
#[instrument(level = "debug", skip(self, sp), ret)]
pub fn try_structurally_resolve_type(&self, sp: Span, ty: Ty<'tcx>) -> Ty<'tcx> {
let ty = self.resolve_vars_with_obligations(ty);

Expand Down
18 changes: 17 additions & 1 deletion compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,23 @@ impl<'a, 'tcx> Trace<'a, 'tcx> {
let Trace { at, trace, a_is_expected } = self;
let mut fields = at.infcx.combine_fields(trace, at.param_env, define_opaque_types);
fields
.equate(a_is_expected)
.equate(StructurallyRelateAliases::No, a_is_expected)
.relate(a, b)
.map(move |_| InferOk { value: (), obligations: fields.obligations })
}

/// Equates `a` and `b` while structurally relating aliases. This should only
/// be used inside of the next generation trait solver when relating rigid aliases.
#[instrument(skip(self), level = "debug")]
pub fn eq_structurally_relating_aliases<T>(self, a: T, b: T) -> InferResult<'tcx, ()>
where
T: Relate<'tcx>,
{
let Trace { at, trace, a_is_expected } = self;
debug_assert!(at.infcx.next_trait_solver());
let mut fields = at.infcx.combine_fields(trace, at.param_env, DefineOpaqueTypes::No);
fields
.equate(StructurallyRelateAliases::Yes, a_is_expected)
.relate(a, b)
.map(move |_| InferOk { value: (), obligations: fields.obligations })
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ use self::region_constraints::{
RegionConstraintCollector, RegionConstraintStorage, RegionSnapshot,
};
pub use self::relate::combine::CombineFields;
pub use self::relate::StructurallyRelateAliases;
use self::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};

pub mod at;
Expand Down
58 changes: 41 additions & 17 deletions compiler/rustc_infer/src/infer/relate/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use super::equate::Equate;
use super::glb::Glb;
use super::lub::Lub;
use super::sub::Sub;
use super::StructurallyRelateAliases;
use crate::infer::{DefineOpaqueTypes, InferCtxt, TypeTrace};
use crate::traits::{Obligation, PredicateObligations};
use rustc_middle::infer::canonical::OriginalQueryValues;
Expand Down Expand Up @@ -116,8 +117,15 @@ impl<'tcx> InferCtxt<'tcx> {
}

(_, ty::Alias(..)) | (ty::Alias(..), _) if self.next_trait_solver() => {
relation.register_type_relate_obligation(a, b);
Ok(a)
match relation.structurally_relate_aliases() {
StructurallyRelateAliases::Yes => {
ty::relate::structurally_relate_tys(relation, a, b)
}
StructurallyRelateAliases::No => {
relation.register_type_relate_obligation(a, b);
Ok(a)
}
}
}

// All other cases of inference are errors
Expand Down Expand Up @@ -240,19 +248,26 @@ impl<'tcx> InferCtxt<'tcx> {
(ty::ConstKind::Unevaluated(..), _) | (_, ty::ConstKind::Unevaluated(..))
if self.tcx.features().generic_const_exprs || self.next_trait_solver() =>
{
let (a, b) = if relation.a_is_expected() { (a, b) } else { (b, a) };

relation.register_predicates([if self.next_trait_solver() {
ty::PredicateKind::AliasRelate(
a.into(),
b.into(),
ty::AliasRelationDirection::Equate,
)
} else {
ty::PredicateKind::ConstEquate(a, b)
}]);

Ok(b)
match relation.structurally_relate_aliases() {
StructurallyRelateAliases::No => {
let (a, b) = if relation.a_is_expected() { (a, b) } else { (b, a) };

relation.register_predicates([if self.next_trait_solver() {
ty::PredicateKind::AliasRelate(
a.into(),
b.into(),
ty::AliasRelationDirection::Equate,
)
} else {
ty::PredicateKind::ConstEquate(a, b)
}]);

Ok(b)
}
StructurallyRelateAliases::Yes => {
ty::relate::structurally_relate_consts(relation, a, b)
}
}
}
_ => ty::relate::structurally_relate_consts(relation, a, b),
}
Expand Down Expand Up @@ -303,8 +318,12 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
self.infcx.tcx
}

pub fn equate<'a>(&'a mut self, a_is_expected: bool) -> Equate<'a, 'infcx, 'tcx> {
Equate::new(self, a_is_expected)
pub fn equate<'a>(
&'a mut self,
structurally_relate_aliases: StructurallyRelateAliases,
a_is_expected: bool,
) -> Equate<'a, 'infcx, 'tcx> {
Equate::new(self, structurally_relate_aliases, a_is_expected)
}

pub fn sub<'a>(&'a mut self, a_is_expected: bool) -> Sub<'a, 'infcx, 'tcx> {
Expand Down Expand Up @@ -335,6 +354,11 @@ pub trait ObligationEmittingRelation<'tcx>: TypeRelation<'tcx> {

fn param_env(&self) -> ty::ParamEnv<'tcx>;

/// Whether aliases should be related structurally. This is pretty much
/// always `No` unless you're equating in some specific locations of the
/// new solver. See the comments in these use-cases for more details.
fn structurally_relate_aliases(&self) -> StructurallyRelateAliases;

/// Register obligations that must hold in order for this relation to hold
fn register_obligations(&mut self, obligations: PredicateObligations<'tcx>);

Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_infer/src/infer/relate/equate.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::combine::{CombineFields, ObligationEmittingRelation};
use super::StructurallyRelateAliases;
use crate::infer::{DefineOpaqueTypes, SubregionOrigin};
use crate::traits::PredicateObligations;

Expand All @@ -13,15 +14,17 @@ use rustc_span::Span;
/// Ensures `a` is made equal to `b`. Returns `a` on success.
pub struct Equate<'combine, 'infcx, 'tcx> {
fields: &'combine mut CombineFields<'infcx, 'tcx>,
structurally_relate_aliases: StructurallyRelateAliases,
a_is_expected: bool,
}

impl<'combine, 'infcx, 'tcx> Equate<'combine, 'infcx, 'tcx> {
pub fn new(
fields: &'combine mut CombineFields<'infcx, 'tcx>,
structurally_relate_aliases: StructurallyRelateAliases,
a_is_expected: bool,
) -> Equate<'combine, 'infcx, 'tcx> {
Equate { fields, a_is_expected }
Equate { fields, structurally_relate_aliases, a_is_expected }
}
}

Expand Down Expand Up @@ -99,7 +102,7 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
&ty::Alias(ty::Opaque, ty::AliasTy { def_id: a_def_id, .. }),
&ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, .. }),
) if a_def_id == b_def_id => {
self.fields.infcx.super_combine_tys(self, a, b)?;
infcx.super_combine_tys(self, a, b)?;
}
(&ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }), _)
| (_, &ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }))
Expand All @@ -120,7 +123,7 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
);
}
_ => {
self.fields.infcx.super_combine_tys(self, a, b)?;
infcx.super_combine_tys(self, a, b)?;
}
}

Expand Down Expand Up @@ -180,6 +183,10 @@ impl<'tcx> ObligationEmittingRelation<'tcx> for Equate<'_, '_, 'tcx> {
self.fields.trace.span()
}

fn structurally_relate_aliases(&self) -> StructurallyRelateAliases {
self.structurally_relate_aliases
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.fields.param_env
}
Expand Down
53 changes: 46 additions & 7 deletions compiler/rustc_infer/src/infer/relate/generalize.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::mem;

use super::StructurallyRelateAliases;
use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind, TypeVariableValue};
use crate::infer::{InferCtxt, ObligationEmittingRelation, RegionVariableOrigin};
use rustc_data_structures::sso::SsoHashMap;
Expand Down Expand Up @@ -45,8 +46,14 @@ impl<'tcx> InferCtxt<'tcx> {
// region/type inference variables.
//
// We then relate `generalized_ty <: source_ty`,adding constraints like `'x: '?2` and `?1 <: ?3`.
let Generalization { value_may_be_infer: generalized_ty, has_unconstrained_ty_var } =
self.generalize(relation.span(), target_vid, instantiation_variance, source_ty)?;
let Generalization { value_may_be_infer: generalized_ty, has_unconstrained_ty_var } = self
.generalize(
relation.span(),
relation.structurally_relate_aliases(),
target_vid,
instantiation_variance,
source_ty,
)?;

// Constrain `b_vid` to the generalized type `generalized_ty`.
if let &ty::Infer(ty::TyVar(generalized_vid)) = generalized_ty.kind() {
Expand Down Expand Up @@ -178,8 +185,14 @@ impl<'tcx> InferCtxt<'tcx> {
) -> RelateResult<'tcx, ()> {
// FIXME(generic_const_exprs): Occurs check failures for unevaluated
// constants and generic expressions are not yet handled correctly.
let Generalization { value_may_be_infer: generalized_ct, has_unconstrained_ty_var } =
self.generalize(relation.span(), target_vid, ty::Variance::Invariant, source_ct)?;
let Generalization { value_may_be_infer: generalized_ct, has_unconstrained_ty_var } = self
.generalize(
relation.span(),
relation.structurally_relate_aliases(),
target_vid,
ty::Variance::Invariant,
source_ct,
)?;

debug_assert!(!generalized_ct.is_ct_infer());
if has_unconstrained_ty_var {
Expand Down Expand Up @@ -217,6 +230,7 @@ impl<'tcx> InferCtxt<'tcx> {
fn generalize<T: Into<Term<'tcx>> + Relate<'tcx>>(
&self,
span: Span,
structurally_relate_aliases: StructurallyRelateAliases,
target_vid: impl Into<ty::TermVid>,
ambient_variance: ty::Variance,
source_term: T,
Expand All @@ -237,6 +251,7 @@ impl<'tcx> InferCtxt<'tcx> {
let mut generalizer = Generalizer {
infcx: self,
span,
structurally_relate_aliases,
root_vid,
for_universe,
ambient_variance,
Expand Down Expand Up @@ -270,6 +285,10 @@ struct Generalizer<'me, 'tcx> {

span: Span,

/// Whether aliases should be related structurally. If not, we have to
/// be careful when generalizing aliases.
structurally_relate_aliases: StructurallyRelateAliases,

/// The vid of the type variable that is in the process of being
/// instantiated. If we find this within the value we are folding,
/// that means we would have created a cyclic value.
Expand Down Expand Up @@ -314,13 +333,30 @@ impl<'tcx> Generalizer<'_, 'tcx> {
/// to normalize the alias after all.
///
/// We handle this by lazily equating the alias and generalizing
/// it to an inference variable.
/// it to an inference variable. In the new solver, we always
/// generalize to an infer var unless the alias contains escaping
/// bound variables.
///
/// This is incomplete and will hopefully soon get fixed by #119106.
/// Correctly handling aliases with escaping bound variables is
/// difficult and currently incomplete in two opposite ways:
/// - if we get an occurs check failure in the alias, replace it with a new infer var.
/// This causes us to later emit an alias-relate goal and is incomplete in case the
/// alias normalizes to type containing one of the bound variables.
/// - if the alias contains an inference variable not nameable by `for_universe`, we
/// continue generalizing the alias. This ends up pulling down the universe of the
/// inference variable and is incomplete in case the alias would normalize to a type
/// which does not mention that inference variable.
fn generalize_alias_ty(
&mut self,
alias: ty::AliasTy<'tcx>,
) -> Result<Ty<'tcx>, TypeError<'tcx>> {
if self.infcx.next_trait_solver() && !alias.has_escaping_bound_vars() {
return Ok(self.infcx.next_ty_var_in_universe(
TypeVariableOrigin { kind: TypeVariableOriginKind::MiscVariable, span: self.span },
self.for_universe,
));
}

let is_nested_alias = mem::replace(&mut self.in_alias, true);
let result = match self.relate(alias, alias) {
Ok(alias) => Ok(alias.to_ty(self.tcx())),
Expand Down Expand Up @@ -490,7 +526,10 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
}
}

ty::Alias(_, data) => self.generalize_alias_ty(data),
ty::Alias(_, data) => match self.structurally_relate_aliases {
StructurallyRelateAliases::No => self.generalize_alias_ty(data),
StructurallyRelateAliases::Yes => relate::structurally_relate_tys(self, t, t),
},

_ => relate::structurally_relate_tys(self, t, t),
}?;
Expand Down
Loading

0 comments on commit b8de591

Please sign in to comment.