Skip to content

Commit

Permalink
Unrolled build for rust-lang#135892
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#135892 - BoxyUwU:next_solver_deduce_sig_normalized, r=lcnr

-Znext-solver: "normalize" signature before checking it mentions self in `deduce_closure_signature`

Fixes rust-lang/trait-system-refactor-initiative#138

r? ``@lcnr``

Tbh I wrote so much comments I don't feel like writing it all again but as a description.
  • Loading branch information
rust-timer authored Jan 28, 2025
2 parents 77a4553 + 356b2aa commit 781f3a8
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 11 deletions.
44 changes: 41 additions & 3 deletions compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

/// Given the expected type, figures out what it can about this closure we
/// are about to type check:
#[instrument(skip(self), level = "debug")]
#[instrument(skip(self), level = "debug", ret)]
fn deduce_closure_signature(
&self,
expected_ty: Ty<'tcx>,
Expand Down Expand Up @@ -378,6 +378,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
bound_predicate.rebind(proj_predicate),
),
);

// Make sure that we didn't infer a signature that mentions itself.
// This can happen when we elaborate certain supertrait bounds that
// mention projections containing the `Self` type. See #105401.
Expand All @@ -395,8 +396,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}
if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
expected_sig = inferred_sig;

// Don't infer a closure signature from a goal that names the closure type as this will
// (almost always) lead to occurs check errors later in type checking.
if self.next_trait_solver()
&& let Some(inferred_sig) = inferred_sig
{
// In the new solver it is difficult to explicitly normalize the inferred signature as we
// would have to manually handle universes and rewriting bound vars and placeholders back
// and forth.
//
// Instead we take advantage of the fact that we relating an inference variable with an alias
// will only instantiate the variable if the alias is rigid(*not quite). Concretely we:
// - Create some new variable `?sig`
// - Equate `?sig` with the unnormalized signature, e.g. `fn(<Foo<?x> as Trait>::Assoc)`
// - Depending on whether `<Foo<?x> as Trait>::Assoc` is rigid, ambiguous or normalizeable,
// we will either wind up with `?sig=<Foo<?x> as Trait>::Assoc/?y/ConcreteTy` respectively.
//
// *: In cases where there are ambiguous aliases in the signature that make use of bound vars
// they will wind up present in `?sig` even though they are non-rigid.
//
// This is a bit weird and means we may wind up discarding the goal due to it naming `expected_ty`
// even though the normalized form may not name `expected_ty`. However, this matches the existing
// behaviour of the old solver and would be technically a breaking change to fix.
let generalized_fnptr_sig = self.next_ty_var(span);
let inferred_fnptr_sig = Ty::new_fn_ptr(self.tcx, inferred_sig.sig);
self.demand_eqtype(span, inferred_fnptr_sig, generalized_fnptr_sig);

let resolved_sig = self.resolve_vars_if_possible(generalized_fnptr_sig);

if resolved_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
expected_sig = Some(ExpectedSig {
cause_span: inferred_sig.cause_span,
sig: resolved_sig.fn_sig(self.tcx),
});
}
} else {
if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
expected_sig = inferred_sig;
}
}
}

Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_span::Span;
use rustc_trait_selection::solve::inspect::{
InspectConfig, InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor,
};
use rustc_type_ir::solve::GoalSource;
use tracing::{debug, instrument, trace};

use crate::FnCtxt;
Expand Down Expand Up @@ -119,7 +120,21 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> {
fn visit_goal(&mut self, inspect_goal: &InspectGoal<'_, 'tcx>) {
let tcx = self.fcx.tcx;
let goal = inspect_goal.goal();
if self.fcx.predicate_has_self_ty(goal.predicate, self.self_ty) {
if self.fcx.predicate_has_self_ty(goal.predicate, self.self_ty)
// We do not push the instantiated forms of goals as it would cause any
// aliases referencing bound vars to go from having escaping bound vars to
// being able to be normalized to an inference variable.
//
// This is mostly just a hack as arbitrary nested goals could still contain
// such aliases while having a different `GoalSource`. Closure signature inference
// however can't really handle *every* higher ranked `Fn` goal also being present
// in the form of `?c: Fn<(<?x as Trait<'!a>>::Assoc)`.
//
// This also just better matches the behaviour of the old solver where we do not
// encounter instantiated forms of goals, only nested goals that referred to bound
// vars from instantiated goals.
&& !matches!(inspect_goal.source(), GoalSource::InstantiateHigherRanked)
{
self.obligations_for_self_ty.push(traits::Obligation::new(
tcx,
self.root_cause.clone(),
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/borrowck/issue-103095.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

trait FnOnceForGenericRef<T>: FnOnce(&T) -> Self::FnOutput {
type FnOutput;
Expand All @@ -16,10 +19,7 @@ struct Data<T, D: FnOnceForGenericRef<T>> {
impl<T, D: FnOnceForGenericRef<T>> Data<T, D> {
fn new(value: T, f: D) -> Self {
let output = f(&value);
Self {
value: Some(value),
output: Some(output),
}
Self { value: Some(value), output: Some(output) }
}
}

Expand Down
3 changes: 3 additions & 0 deletions tests/ui/closures/supertrait-hint-references-assoc-ty.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

pub trait Fn0: Fn(i32) -> Self::Out {
type Out;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ fn main() {
let _ = (-10..=10).find(|x: &i32| x.signum() == 0);
//[current]~^ ERROR type mismatch in closure arguments
//[next]~^^ ERROR expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
//[next]~| ERROR expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,26 @@ note: required by a bound in `find`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL

error[E0271]: expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
--> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:33
--> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:24
|
LL | let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
| ^^^^^^ expected `&&i32`, found integer
| ^^^^ expected `&&i32`, found integer

error: aborting due to 2 previous errors
error[E0277]: expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
--> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:29
|
LL | let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
| ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
| |
| required by a bound introduced by this call
|
= help: the trait `for<'a> FnMut(&'a <RangeInclusive<{integer}> as Iterator>::Item)` is not implemented for closure `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
= note: expected a closure with arguments `(&&&i32,)`
found a closure with arguments `(&<RangeInclusive<{integer}> as Iterator>::Item,)`
note: required by a bound in `find`
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0271, E0277.
For more information about an error, try `rustc --explain E0271`.
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ fn main() {
let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
//[current]~^ ERROR type mismatch in closure arguments
//[next]~^^ ERROR expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
//[next]~| ERROR expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//@ check-pass
//@ revisions: current next
//@[next] compile-flags: -Znext-solver

// When type checking a closure expr we look at the list of unsolved goals
// to determine if there are any bounds on the closure type to infer a signature from.
//
// We attempt to discard goals that name the closure type so as to avoid inferring the
// closure type to something like `?x = closure(sig=fn(?x))`. This test checks that when
// such a goal names the closure type inside of an ambiguous alias and there exists another
// potential goal to infer the closure signature from, we do that.

trait Trait<'a> {
type Assoc;
}

impl<'a, F> Trait<'a> for F {
type Assoc = u32;
}

fn closure_typer1<F>(_: F)
where
F: Fn(u32) + for<'a> Fn(<F as Trait<'a>>::Assoc),
{
}

fn closure_typer2<F>(_: F)
where
F: for<'a> Fn(<F as Trait<'a>>::Assoc) + Fn(u32),
{
}

fn main() {
// Here we have some closure with a yet to be inferred type of `?c`. There are two goals
// involving `?c` that can be used to determine the closure signature:
// - `?c: for<'a> Fn<(<?c as Trait<'a>>::Assoc,), Output = ()>`
// - `?c: Fn<(u32,), Output = ()>`
//
// If we were to infer the argument of the closure (`x` below) to `<?c as Trait<'a>>::Assoc`
// then we would not be able to call `x.into()` as `x` is some unknown type. Instead we must
// use the `?c: Fn(u32)` goal to infer a signature in order for this code to compile.
//
// As the algorithm for picking a goal to infer the signature from is dependent on the ordering
// of pending goals in the type checker, we test both orderings of bounds to ensure we aren't
// testing that we just *happen* to pick `?c: Fn(u32)`.
closure_typer1(move |x| {
let _: u32 = x.into();
});
closure_typer2(move |x| {
let _: u32 = x.into();
});
}

0 comments on commit 781f3a8

Please # to comment.