Skip to content

Commit

Permalink
generalize hr alias: avoid unconstrainable infer vars
Browse files Browse the repository at this point in the history
  • Loading branch information
lcnr committed May 7, 2024
1 parent 3111015 commit 690d5aa
Show file tree
Hide file tree
Showing 7 changed files with 175 additions and 26 deletions.
32 changes: 11 additions & 21 deletions compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ struct ExpectedSig<'tcx> {
sig: ty::PolyFnSig<'tcx>,
}

#[derive(Debug)]
struct ClosureSignatures<'tcx> {
/// The signature users of the closure see.
bound_sig: ty::PolyFnSig<'tcx>,
Expand Down Expand Up @@ -713,25 +714,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// [c2]: https://github.com/rust-lang/rust/pull/45072#issuecomment-341096796
self.commit_if_ok(|_| {
let mut all_obligations = vec![];
let inputs: Vec<_> = iter::zip(
decl.inputs,
supplied_sig.inputs().skip_binder(), // binder moved to (*) below
)
.map(|(hir_ty, &supplied_ty)| {
// Instantiate (this part of..) S to S', i.e., with fresh variables.
self.instantiate_binder_with_fresh_vars(
hir_ty.span,
BoundRegionConversionTime::FnCall,
// (*) binder moved to here
supplied_sig.inputs().rebind(supplied_ty),
)
})
.collect();
let supplied_sig = self.instantiate_binder_with_fresh_vars(
self.tcx.def_span(expr_def_id),
BoundRegionConversionTime::FnCall,
supplied_sig,
);

// The liberated version of this signature should be a subtype
// of the liberated form of the expectation.
for ((hir_ty, &supplied_ty), expected_ty) in iter::zip(
iter::zip(decl.inputs, &inputs),
iter::zip(decl.inputs, supplied_sig.inputs()),
expected_sigs.liberated_sig.inputs(), // `liberated_sig` is E'.
) {
// Check that E' = S'.
Expand All @@ -744,11 +736,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
all_obligations.extend(obligations);
}

let supplied_output_ty = self.instantiate_binder_with_fresh_vars(
decl.output.span(),
BoundRegionConversionTime::FnCall,
supplied_sig.output(),
);
let supplied_output_ty = supplied_sig.output();
let cause = &self.misc(decl.output.span());
let InferOk { value: (), obligations } = self.at(cause, self.param_env).eq(
DefineOpaqueTypes::Yes,
Expand All @@ -757,7 +745,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)?;
all_obligations.extend(obligations);

let inputs = inputs.into_iter().map(|ty| self.resolve_vars_if_possible(ty));
let inputs =
supplied_sig.inputs().into_iter().map(|&ty| self.resolve_vars_if_possible(ty));

expected_sigs.liberated_sig = self.tcx.mk_fn_sig(
inputs,
Expand Down Expand Up @@ -1013,6 +1002,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
result
}

#[instrument(level = "debug", skip(self), ret)]
fn closure_sigs(
&self,
expr_def_id: LocalDefId,
Expand Down
44 changes: 40 additions & 4 deletions compiler/rustc_infer/src/infer/relate/generalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,13 @@ impl<'tcx> Generalizer<'_, 'tcx> {
&mut self,
alias: ty::AliasTy<'tcx>,
) -> Result<Ty<'tcx>, TypeError<'tcx>> {
if self.infcx.next_trait_solver() && !alias.has_escaping_bound_vars() {
// We do not eagerly replace aliases with inference variables if they have
// escaping bound vars, see the method comment for details. However, when we
// are inside of an alias with escaping bound vars replacing nested aliases
// with inference variables can cause incorrect ambiguity.
//
// cc trait-system-refactor-initiative#110
if self.infcx.next_trait_solver() && !alias.has_escaping_bound_vars() && !self.in_alias {
return Ok(self.infcx.next_ty_var_in_universe(
TypeVariableOrigin { param_def_id: None, span: self.span },
self.for_universe,
Expand Down Expand Up @@ -492,9 +498,30 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
let origin = inner.type_variables().var_origin(vid);
let new_var_id =
inner.type_variables().new_var(self.for_universe, origin);
let u = Ty::new_var(self.tcx(), new_var_id);
debug!("replacing original vid={:?} with new={:?}", vid, u);
Ok(u)
// If we're in the new solver and create a new inference
// variable inside of an alias we eagerly constrain that
// inference variable to prevent unexpected ambiguity errors.
//
// This is incomplete as it pulls down the universe of the
// original inference variable, even though the alias could
// normalize to a type which does not refer to that type at
// all. I don't expect this to cause unexpected errors in
// practice.
//
// We only need to do so for type and const variables, as
// region variables do not impact normalization, and will get
// correctly constrained by `AliasRelate` later on.
//
// cc trait-system-refactor-initiative#108
if self.infcx.next_trait_solver()
&& !self.infcx.intercrate
&& self.in_alias
{
inner.type_variables().equate(vid, new_var_id);
}

debug!("replacing original vid={:?} with new={:?}", vid, new_var_id);
Ok(Ty::new_var(self.tcx(), new_var_id))
}
}
}
Expand Down Expand Up @@ -614,6 +641,15 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
universe: self.for_universe,
})
.vid;

// See the comment for type inference variables
// for more details.
if self.infcx.next_trait_solver()
&& !self.infcx.intercrate
&& self.in_alias
{
variable_table.union(vid, new_var_id);
}
Ok(ty::Const::new_var(self.tcx(), new_var_id, c.ty()))
}
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_trait_selection/src/solve/inspect/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,10 @@ impl<'tcx> ProofTreeBuilder<'tcx> {
(
DebugSolver::GoalEvaluation(goal_evaluation),
DebugSolver::CanonicalGoalEvaluation(canonical_goal_evaluation),
) => goal_evaluation.evaluation = Some(canonical_goal_evaluation),
) => {
let prev = goal_evaluation.evaluation.replace(canonical_goal_evaluation);
assert_eq!(prev, None);
}
_ => unreachable!(),
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//@ revisions: old next
//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass

// Generalizing an alias referencing escaping bound variables
// is hard. We previously didn't replace this alias with inference
// variables but did replace nested alias which do not reference
// any bound variables. This caused us to stall with the following
// goal, which cannot make any progress:
//
// <<T as Id>::Refl as HigherRanked>::Output<'a>
// alias-relate
// <?unconstrained as HigherRanked>::Output<'a>
//
//
// cc trait-system-refactor-initiative#110

#![allow(unused)]
trait HigherRanked {
type Output<'a>;
}
trait Id {
type Refl: HigherRanked;
}

fn foo<T: Id>() -> for<'a> fn(<<T as Id>::Refl as HigherRanked>::Output<'a>) {
todo!()
}

fn bar<T: Id>() {
// we generalize here
let x = foo::<T>();
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//@ revisions: old next
//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass

// A minimization of an ambiguity error in `icu_provider`.
//
// cc trait-system-refactor-initiative#110

trait Yokeable<'a> {
type Output;
}
trait Id {
type Refl;
}

fn into_deserialized<M: Id>() -> M
where
M::Refl: for<'a> Yokeable<'a>,
{
try_map_project::<M, _>(|_| todo!())
}

fn try_map_project<M: Id, F>(_f: F) -> M
where
M::Refl: for<'a> Yokeable<'a>,
F: for<'a> FnOnce(&'a ()) -> <<M as Id>::Refl as Yokeable<'a>>::Output,
{
todo!()
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//@ compile-flags: -Znext-solver
//@ check-pass

// A regression test for a fairly subtle issue with how we
// generalize aliases referencing higher-ranked regions
// which previously caused unexpected ambiguity errors.
//
// The explanations in the test may end up being out of date
// in the future as we may refine our approach to generalization
// going forward.
//
// cc trait-system-refactor-initiative#108
trait Trait<'a> {
type Assoc;
}

impl<'a> Trait<'a> for () {
type Assoc = ();
}

fn foo<T: for<'a> Trait<'a>>(x: T) -> for<'a> fn(<T as Trait<'a>>::Assoc) {
|_| ()
}

fn unconstrained<T>() -> T {
todo!()
}

fn main() {
// create `?x.0` in the root universe
let mut x = unconstrained();

// bump the current universe of the inference context
let bump: for<'a, 'b> fn(&'a (), &'b ()) = |_, _| ();
let _: for<'a> fn(&'a (), &'a ()) = bump;

// create `?y.1` in a higher universe
let mut y = Default::default();

// relate `?x.0` with `for<'a> fn(<?y.1 as Trait<'a>>::Assoc)`
// -> instantiate `?x.0` with `for<'a> fn(<?y_new.0 as Trait<'a>>::Assoc)`
x = foo(y);

// Constrain `?y.1` to `()`
let _: () = y;

// `AliasRelate(<?y_new.0 as Trait<'a>>::Assoc, <() as Trait<'a>>::Assoc)`
// remains ambiguous unless we somehow constrain `?y_new.0` during
// generalization to be equal to `?y.1`, which is exactly what we
// did to fix this issue.
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ revisions: old next
//@[next] compile-flags: -Znext-solver
//@ ignore-compare-mode-next-solver (explicit revisions)
//@ check-pass

// case 3 of https://github.com/rust-lang/trait-system-refactor-initiative/issues/8.
Expand Down

0 comments on commit 690d5aa

Please sign in to comment.