Skip to content

Commit

Permalink
Auto merge of rust-lang#121211 - lcnr:nll-relate-handle-infer, r=BoxyUwU
Browse files Browse the repository at this point in the history 
deduplicate infer var instantiation

Having 3 separate implementations of one of the most subtle parts of our type system is not a good strategy if we want to maintain a sound type system ✨ while working on this I already found some subtle bugs in the existing code, so that's awesome 🎉 cc rust-lang#121159

This was necessary as I am not confident in my nll changes in rust-lang#119106, so I am first cleaning this up in a separate PR.

r? `@BoxyUwU`
  • Loading branch information
@bors
bors committed Feb 19, 2024
2 parents 3246e79 + 54b3c87 commit 0395fa3
Show file tree
Hide file tree
Showing 15 changed files with 345 additions and 566 deletions.
16 changes: 0 additions & 16 deletions compiler/rustc_borrowck/src/type_check/relate_tys.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,22 +143,6 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx>
reg
}

#[instrument(skip(self), level = "debug")]
fn generalize_existential(&mut self, universe: ty::UniverseIndex) -> ty::Region<'tcx> {
let reg = self.type_checker.infcx.next_nll_region_var_in_universe(
NllRegionVariableOrigin::Existential { from_forall: false },
universe,
);

if cfg!(debug_assertions) {
let mut var_to_origin = self.type_checker.infcx.reg_var_to_origin.borrow_mut();
let prev = var_to_origin.insert(reg.as_var(), RegionCtxt::Existential(None));
assert_eq!(prev, None);
}

reg
}

fn push_outlives(
&mut self,
sup: ty::Region<'tcx>,
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_infer/src/infer/canonical/query_response.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,13 +731,6 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for QueryTypeRelatingDelegate<'_, 'tcx> {
ty::Region::new_placeholder(self.infcx.tcx, placeholder)
}

fn generalize_existential(&mut self, universe: ty::UniverseIndex) -> ty::Region<'tcx> {
self.infcx.next_nll_region_var_in_universe(
NllRegionVariableOrigin::Existential { from_forall: false },
universe,
)
}

fn push_outlives(
&mut self,
sup: ty::Region<'tcx>,
Expand Down
199 changes: 6 additions & 193 deletions compiler/rustc_infer/src/infer/relate/combine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,18 @@
//! this should be correctly updated.

use super::equate::Equate;
use super::generalize::{self, CombineDelegate, Generalization};
use super::glb::Glb;
use super::lub::Lub;
use super::sub::Sub;
use crate::infer::{DefineOpaqueTypes, InferCtxt, TypeTrace};
use crate::traits::{Obligation, PredicateObligations};
use rustc_middle::infer::canonical::OriginalQueryValues;
use rustc_middle::infer::unify_key::{ConstVariableValue, EffectVarValue};
use rustc_middle::infer::unify_key::EffectVarValue;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::relate::{RelateResult, TypeRelation};
use rustc_middle::ty::{self, InferConst, ToPredicate, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{AliasRelationDirection, TyVar};
use rustc_middle::ty::{IntType, UintType};
use rustc_span::Span;

#[derive(Clone)]
pub struct CombineFields<'infcx, 'tcx> {
Expand Down Expand Up @@ -221,11 +220,11 @@ impl<'tcx> InferCtxt<'tcx> {
}

(ty::ConstKind::Infer(InferConst::Var(vid)), _) => {
return self.unify_const_variable(vid, b);
return self.instantiate_const_var(vid, b);
}

(_, ty::ConstKind::Infer(InferConst::Var(vid))) => {
return self.unify_const_variable(vid, a);
return self.instantiate_const_var(vid, a);
}

(ty::ConstKind::Infer(InferConst::EffectVar(vid)), _) => {
Expand Down Expand Up @@ -259,69 +258,6 @@ impl<'tcx> InferCtxt<'tcx> {
ty::relate::structurally_relate_consts(relation, a, b)
}

/// Unifies the const variable `target_vid` with the given constant.
///
/// This also tests if the given const `ct` contains an inference variable which was previously
/// unioned with `target_vid`. If this is the case, inferring `target_vid` to `ct`
/// would result in an infinite type as we continuously replace an inference variable
/// in `ct` with `ct` itself.
///
/// This is especially important as unevaluated consts use their parents generics.
/// They therefore often contain unused args, making these errors far more likely.
///
/// A good example of this is the following:
///
/// ```compile_fail,E0308
/// #![feature(generic_const_exprs)]
///
/// fn bind<const N: usize>(value: [u8; N]) -> [u8; 3 + 4] {
/// todo!()
/// }
///
/// fn main() {
/// let mut arr = Default::default();
/// arr = bind(arr);
/// }
/// ```
///
/// Here `3 + 4` ends up as `ConstKind::Unevaluated` which uses the generics
/// of `fn bind` (meaning that its args contain `N`).
///
/// `bind(arr)` now infers that the type of `arr` must be `[u8; N]`.
/// The assignment `arr = bind(arr)` now tries to equate `N` with `3 + 4`.
///
/// As `3 + 4` contains `N` in its args, this must not succeed.
///
/// See `tests/ui/const-generics/occurs-check/` for more examples where this is relevant.
#[instrument(level = "debug", skip(self))]
fn unify_const_variable(
&self,
target_vid: ty::ConstVid,
ct: ty::Const<'tcx>,
) -> RelateResult<'tcx, ty::Const<'tcx>> {
let span = match self.inner.borrow_mut().const_unification_table().probe_value(target_vid) {
ConstVariableValue::Known { value } => {
bug!("instantiating a known const var: {target_vid:?} {value} {ct}")
}
ConstVariableValue::Unknown { origin, universe: _ } => origin.span,
};
// FIXME(generic_const_exprs): Occurs check failures for unevaluated
// constants and generic expressions are not yet handled correctly.
let Generalization { value_may_be_infer: value, needs_wf: _ } = generalize::generalize(
self,
&mut CombineDelegate { infcx: self, span },
ct,
target_vid,
ty::Variance::Invariant,
)?;

self.inner
.borrow_mut()
.const_unification_table()
.union_value(target_vid, ConstVariableValue::Known { value });
Ok(value)
}

fn unify_integral_variable(
&self,
vid_is_expected: bool,
Expand Down Expand Up @@ -383,131 +319,6 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
Glb::new(self, a_is_expected)
}

/// Here, `dir` is either `EqTo`, `SubtypeOf`, or `SupertypeOf`.
/// The idea is that we should ensure that the type `a_ty` is equal
/// to, a subtype of, or a supertype of (respectively) the type
/// to which `b_vid` is bound.
///
/// Since `b_vid` has not yet been instantiated with a type, we
/// will first instantiate `b_vid` with a *generalized* version
/// of `a_ty`. Generalization introduces other inference
/// variables wherever subtyping could occur.
#[instrument(skip(self), level = "debug")]
pub fn instantiate(
&mut self,
a_ty: Ty<'tcx>,
ambient_variance: ty::Variance,
b_vid: ty::TyVid,
a_is_expected: bool,
) -> RelateResult<'tcx, ()> {
// Get the actual variable that b_vid has been inferred to
debug_assert!(self.infcx.inner.borrow_mut().type_variables().probe(b_vid).is_unknown());

// Generalize type of `a_ty` appropriately depending on the
// direction. As an example, assume:
//
// - `a_ty == &'x ?1`, where `'x` is some free region and `?1` is an
// inference variable,
// - and `dir` == `SubtypeOf`.
//
// Then the generalized form `b_ty` would be `&'?2 ?3`, where
// `'?2` and `?3` are fresh region/type inference
// variables. (Down below, we will relate `a_ty <: b_ty`,
// adding constraints like `'x: '?2` and `?1 <: ?3`.)
let Generalization { value_may_be_infer: b_ty, needs_wf } = generalize::generalize(
self.infcx,
&mut CombineDelegate { infcx: self.infcx, span: self.trace.span() },
a_ty,
b_vid,
ambient_variance,
)?;

// Constrain `b_vid` to the generalized type `b_ty`.
if let &ty::Infer(TyVar(b_ty_vid)) = b_ty.kind() {
self.infcx.inner.borrow_mut().type_variables().equate(b_vid, b_ty_vid);
} else {
self.infcx.inner.borrow_mut().type_variables().instantiate(b_vid, b_ty);
}

if needs_wf {
self.obligations.push(Obligation::new(
self.tcx(),
self.trace.cause.clone(),
self.param_env,
ty::Binder::dummy(ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(
b_ty.into(),
))),
));
}

// Finally, relate `b_ty` to `a_ty`, as described in previous comment.
//
// FIXME(#16847): This code is non-ideal because all these subtype
// relations wind up attributed to the same spans. We need
// to associate causes/spans with each of the relations in
// the stack to get this right.
if b_ty.is_ty_var() {
// This happens for cases like `<?0 as Trait>::Assoc == ?0`.
// We can't instantiate `?0` here as that would result in a
// cyclic type. We instead delay the unification in case
// the alias can be normalized to something which does not
// mention `?0`.
if self.infcx.next_trait_solver() {
let (lhs, rhs, direction) = match ambient_variance {
ty::Variance::Invariant => {
(a_ty.into(), b_ty.into(), AliasRelationDirection::Equate)
}
ty::Variance::Covariant => {
(a_ty.into(), b_ty.into(), AliasRelationDirection::Subtype)
}
ty::Variance::Contravariant => {
(b_ty.into(), a_ty.into(), AliasRelationDirection::Subtype)
}
ty::Variance::Bivariant => unreachable!("bivariant generalization"),
};
self.obligations.push(Obligation::new(
self.tcx(),
self.trace.cause.clone(),
self.param_env,
ty::PredicateKind::AliasRelate(lhs, rhs, direction),
));
} else {
match a_ty.kind() {
&ty::Alias(ty::Projection, data) => {
// FIXME: This does not handle subtyping correctly, we could
// instead create a new inference variable for `a_ty`, emitting
// `Projection(a_ty, a_infer)` and `a_infer <: b_ty`.
self.obligations.push(Obligation::new(
self.tcx(),
self.trace.cause.clone(),
self.param_env,
ty::ProjectionPredicate { projection_ty: data, term: b_ty.into() },
))
}
// The old solver only accepts projection predicates for associated types.
ty::Alias(ty::Inherent | ty::Weak | ty::Opaque, _) => {
return Err(TypeError::CyclicTy(a_ty));
}
_ => bug!("generalizated `{a_ty:?} to infer, not an alias"),
}
}
} else {
match ambient_variance {
ty::Variance::Invariant => self.equate(a_is_expected).relate(a_ty, b_ty),
ty::Variance::Covariant => self.sub(a_is_expected).relate(a_ty, b_ty),
ty::Variance::Contravariant => self.sub(a_is_expected).relate_with_variance(
ty::Contravariant,
ty::VarianceDiagInfo::default(),
a_ty,
b_ty,
),
ty::Variance::Bivariant => unreachable!("bivariant generalization"),
}?;
}

Ok(())
}

pub fn register_obligations(&mut self, obligations: PredicateObligations<'tcx>) {
self.obligations.extend(obligations);
}
Expand All @@ -520,6 +331,8 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
}

pub trait ObligationEmittingRelation<'tcx>: TypeRelation<'tcx> {
fn span(&self) -> Span;

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

/// Register obligations that must hold in order for this relation to hold
Expand Down
13 changes: 9 additions & 4 deletions compiler/rustc_infer/src/infer/relate/equate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_middle::ty::TyVar;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};

use rustc_hir::def_id::DefId;
use rustc_span::Span;

/// Ensures `a` is made equal to `b`. Returns `a` on success.
pub struct Equate<'combine, 'infcx, 'tcx> {
Expand Down Expand Up @@ -81,12 +82,12 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
infcx.inner.borrow_mut().type_variables().equate(a_id, b_id);
}

(&ty::Infer(TyVar(a_id)), _) => {
self.fields.instantiate(b, ty::Invariant, a_id, self.a_is_expected)?;
(&ty::Infer(TyVar(a_vid)), _) => {
infcx.instantiate_ty_var(self, self.a_is_expected, a_vid, ty::Invariant, b)?;
}

(_, &ty::Infer(TyVar(b_id))) => {
self.fields.instantiate(a, ty::Invariant, b_id, self.a_is_expected)?;
(_, &ty::Infer(TyVar(b_vid))) => {
infcx.instantiate_ty_var(self, !self.a_is_expected, b_vid, ty::Invariant, a)?;
}

(
Expand Down Expand Up @@ -170,6 +171,10 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
}

impl<'tcx> ObligationEmittingRelation<'tcx> for Equate<'_, '_, 'tcx> {
fn span(&self) -> Span {
self.fields.trace.span()
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.fields.param_env
}
Expand Down
Loading

0 comments on commit 0395fa3

Please sign in to comment.