Skip to content

Commit

Permalink
"normalize" signature before checking mentions self
Browse files Browse the repository at this point in the history
  • Loading branch information
BoxyUwU committed Jan 22, 2025
1 parent b2728d5 commit 433c887
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 8 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
@@ -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 433c887

Please sign in to comment.