Skip to content

Commit 356b2aa

Browse files
committed
"normalize" signature before checking mentions self
1 parent aa6f5ab commit 356b2aa

8 files changed

+135
-11
lines changed

compiler/rustc_hir_typeck/src/closure.rs

+41-3
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
296296

297297
/// Given the expected type, figures out what it can about this closure we
298298
/// are about to type check:
299-
#[instrument(skip(self), level = "debug")]
299+
#[instrument(skip(self), level = "debug", ret)]
300300
fn deduce_closure_signature(
301301
&self,
302302
expected_ty: Ty<'tcx>,
@@ -378,6 +378,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
378378
bound_predicate.rebind(proj_predicate),
379379
),
380380
);
381+
381382
// Make sure that we didn't infer a signature that mentions itself.
382383
// This can happen when we elaborate certain supertrait bounds that
383384
// mention projections containing the `Self` type. See #105401.
@@ -395,8 +396,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
395396
}
396397
}
397398
}
398-
if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
399-
expected_sig = inferred_sig;
399+
400+
// Don't infer a closure signature from a goal that names the closure type as this will
401+
// (almost always) lead to occurs check errors later in type checking.
402+
if self.next_trait_solver()
403+
&& let Some(inferred_sig) = inferred_sig
404+
{
405+
// In the new solver it is difficult to explicitly normalize the inferred signature as we
406+
// would have to manually handle universes and rewriting bound vars and placeholders back
407+
// and forth.
408+
//
409+
// Instead we take advantage of the fact that we relating an inference variable with an alias
410+
// will only instantiate the variable if the alias is rigid(*not quite). Concretely we:
411+
// - Create some new variable `?sig`
412+
// - Equate `?sig` with the unnormalized signature, e.g. `fn(<Foo<?x> as Trait>::Assoc)`
413+
// - Depending on whether `<Foo<?x> as Trait>::Assoc` is rigid, ambiguous or normalizeable,
414+
// we will either wind up with `?sig=<Foo<?x> as Trait>::Assoc/?y/ConcreteTy` respectively.
415+
//
416+
// *: In cases where there are ambiguous aliases in the signature that make use of bound vars
417+
// they will wind up present in `?sig` even though they are non-rigid.
418+
//
419+
// This is a bit weird and means we may wind up discarding the goal due to it naming `expected_ty`
420+
// even though the normalized form may not name `expected_ty`. However, this matches the existing
421+
// behaviour of the old solver and would be technically a breaking change to fix.
422+
let generalized_fnptr_sig = self.next_ty_var(span);
423+
let inferred_fnptr_sig = Ty::new_fn_ptr(self.tcx, inferred_sig.sig);
424+
self.demand_eqtype(span, inferred_fnptr_sig, generalized_fnptr_sig);
425+
426+
let resolved_sig = self.resolve_vars_if_possible(generalized_fnptr_sig);
427+
428+
if resolved_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
429+
expected_sig = Some(ExpectedSig {
430+
cause_span: inferred_sig.cause_span,
431+
sig: resolved_sig.fn_sig(self.tcx),
432+
});
433+
}
434+
} else {
435+
if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
436+
expected_sig = inferred_sig;
437+
}
400438
}
401439
}
402440

compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use rustc_span::Span;
77
use rustc_trait_selection::solve::inspect::{
88
InspectConfig, InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor,
99
};
10+
use rustc_type_ir::solve::GoalSource;
1011
use tracing::{debug, instrument, trace};
1112

1213
use crate::FnCtxt;
@@ -119,7 +120,21 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> {
119120
fn visit_goal(&mut self, inspect_goal: &InspectGoal<'_, 'tcx>) {
120121
let tcx = self.fcx.tcx;
121122
let goal = inspect_goal.goal();
122-
if self.fcx.predicate_has_self_ty(goal.predicate, self.self_ty) {
123+
if self.fcx.predicate_has_self_ty(goal.predicate, self.self_ty)
124+
// We do not push the instantiated forms of goals as it would cause any
125+
// aliases referencing bound vars to go from having escaping bound vars to
126+
// being able to be normalized to an inference variable.
127+
//
128+
// This is mostly just a hack as arbitrary nested goals could still contain
129+
// such aliases while having a different `GoalSource`. Closure signature inference
130+
// however can't really handle *every* higher ranked `Fn` goal also being present
131+
// in the form of `?c: Fn<(<?x as Trait<'!a>>::Assoc)`.
132+
//
133+
// This also just better matches the behaviour of the old solver where we do not
134+
// encounter instantiated forms of goals, only nested goals that referred to bound
135+
// vars from instantiated goals.
136+
&& !matches!(inspect_goal.source(), GoalSource::InstantiateHigherRanked)
137+
{
123138
self.obligations_for_self_ty.push(traits::Obligation::new(
124139
tcx,
125140
self.root_cause.clone(),

tests/ui/borrowck/issue-103095.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
//@ check-pass
2+
//@ revisions: current next
3+
//@ ignore-compare-mode-next-solver (explicit revisions)
4+
//@[next] compile-flags: -Znext-solver
25

36
trait FnOnceForGenericRef<T>: FnOnce(&T) -> Self::FnOutput {
47
type FnOutput;
@@ -16,10 +19,7 @@ struct Data<T, D: FnOnceForGenericRef<T>> {
1619
impl<T, D: FnOnceForGenericRef<T>> Data<T, D> {
1720
fn new(value: T, f: D) -> Self {
1821
let output = f(&value);
19-
Self {
20-
value: Some(value),
21-
output: Some(output),
22-
}
22+
Self { value: Some(value), output: Some(output) }
2323
}
2424
}
2525

tests/ui/closures/supertrait-hint-references-assoc-ty.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
//@ check-pass
2+
//@ revisions: current next
3+
//@ ignore-compare-mode-next-solver (explicit revisions)
4+
//@[next] compile-flags: -Znext-solver
25

36
pub trait Fn0: Fn(i32) -> Self::Out {
47
type Out;

tests/ui/mismatched_types/closure-arg-type-mismatch-issue-45727.current.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ fn main() {
99
let _ = (-10..=10).find(|x: &i32| x.signum() == 0);
1010
//[current]~^ ERROR type mismatch in closure arguments
1111
//[next]~^^ ERROR expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
12+
//[next]~| ERROR expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found
1213
}

tests/ui/mismatched_types/closure-arg-type-mismatch-issue-45727.next.stderr

+17-3
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,26 @@ note: required by a bound in `find`
1313
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
1414

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

21-
error: aborting due to 2 previous errors
21+
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}`
22+
--> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:29
23+
|
24+
LL | let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
25+
| ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
26+
| |
27+
| required by a bound introduced by this call
28+
|
29+
= 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}`
30+
= note: expected a closure with arguments `(&&&i32,)`
31+
found a closure with arguments `(&<RangeInclusive<{integer}> as Iterator>::Item,)`
32+
note: required by a bound in `find`
33+
--> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
34+
35+
error: aborting due to 3 previous errors
2236

2337
Some errors have detailed explanations: E0271, E0277.
2438
For more information about an error, try `rustc --explain E0271`.

tests/ui/mismatched_types/closure-arg-type-mismatch-issue-45727.rs

+1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,5 @@ fn main() {
99
let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
1010
//[current]~^ ERROR type mismatch in closure arguments
1111
//[next]~^^ ERROR expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
12+
//[next]~| ERROR expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found
1213
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
//@ check-pass
2+
//@ revisions: current next
3+
//@[next] compile-flags: -Znext-solver
4+
5+
// When type checking a closure expr we look at the list of unsolved goals
6+
// to determine if there are any bounds on the closure type to infer a signature from.
7+
//
8+
// We attempt to discard goals that name the closure type so as to avoid inferring the
9+
// closure type to something like `?x = closure(sig=fn(?x))`. This test checks that when
10+
// such a goal names the closure type inside of an ambiguous alias and there exists another
11+
// potential goal to infer the closure signature from, we do that.
12+
13+
trait Trait<'a> {
14+
type Assoc;
15+
}
16+
17+
impl<'a, F> Trait<'a> for F {
18+
type Assoc = u32;
19+
}
20+
21+
fn closure_typer1<F>(_: F)
22+
where
23+
F: Fn(u32) + for<'a> Fn(<F as Trait<'a>>::Assoc),
24+
{
25+
}
26+
27+
fn closure_typer2<F>(_: F)
28+
where
29+
F: for<'a> Fn(<F as Trait<'a>>::Assoc) + Fn(u32),
30+
{
31+
}
32+
33+
fn main() {
34+
// Here we have some closure with a yet to be inferred type of `?c`. There are two goals
35+
// involving `?c` that can be used to determine the closure signature:
36+
// - `?c: for<'a> Fn<(<?c as Trait<'a>>::Assoc,), Output = ()>`
37+
// - `?c: Fn<(u32,), Output = ()>`
38+
//
39+
// If we were to infer the argument of the closure (`x` below) to `<?c as Trait<'a>>::Assoc`
40+
// then we would not be able to call `x.into()` as `x` is some unknown type. Instead we must
41+
// use the `?c: Fn(u32)` goal to infer a signature in order for this code to compile.
42+
//
43+
// As the algorithm for picking a goal to infer the signature from is dependent on the ordering
44+
// of pending goals in the type checker, we test both orderings of bounds to ensure we aren't
45+
// testing that we just *happen* to pick `?c: Fn(u32)`.
46+
closure_typer1(move |x| {
47+
let _: u32 = x.into();
48+
});
49+
closure_typer2(move |x| {
50+
let _: u32 = x.into();
51+
});
52+
}

0 commit comments

Comments
 (0)