Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)`.
Comment on lines +129 to +131
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment feels a bit detached from where closure signature inference is located xd

//
// 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();
});
}
Loading