Skip to content

Commit 35a31ba

Browse files
committed
Auto merge of rust-lang#140283 - adwinwhite:fn-pointer-coercion, r=jackh726
Fix accidental type inference in array coercion Fixes rust-lang#136420. If the expectation of array element is a type variable, we should avoid resolving it to the first element's type and wait until LUB coercion is completed. We create a free type variable instead which is only used in this `CoerceMany`. [`check_expr_match`](https://github.com/rust-lang/rust/blob/847e3ee6b0e614937eee4e6d8f61094411eadcc0/compiler/rustc_hir_typeck/src/_match.rs#L72) and [`check_expr_if`](https://github.com/rust-lang/rust/blob/847e3ee6b0e614937eee4e6d8f61094411eadcc0/compiler/rustc_hir_typeck/src/expr.rs#L1329) where `CoerceMany` is also used do the [same](https://github.com/rust-lang/rust/blob/847e3ee6b0e614937eee4e6d8f61094411eadcc0/compiler/rustc_hir_typeck/src/expectation.rs#L50). ### [FCP Proposal](rust-lang#140283 (comment)): > Array expressions normally lub their element expressions' types to ensure that things like `[5, 5_u8]` work and don't result in type mismatches. When invoking a generic function `fn foo<T>(_: [T; N])` with an array expression, we end up with an infer var for the element type of the array in the signature. So when typecking the first array element we compare its type with the infer var and thus subsequently require all other elements to be the same type. > > This PR changes that to instead fall back to "not knowing" that the argument type is array of infer var, but just having an infer var for the entire argument. Thus we typeck the array expression normally, lubbing the element expressions, and then in the end comparing the array expression's type with the array of infer var type. > > Things like > > ```rust > fn foo() {} > fn bar() {} > fn f<T>(_: [T; 2]) {} > > f([foo, bar]); > ``` > > and > > ```rust > struct Foo; > struct Bar; > trait Trait {} > impl Trait for Foo {} > impl Trait for Bar {} > fn f<T>(_: [T; 2]) {} > > f([&Foo, &Bar as &dyn Trait]); > ``` ### Remaining inconsistency with `if` and `match`(rust-lang#145048): The typeck of array always uses the element coercion target type as the expectation of element exprs while `if` and `match` use `NoExpectation` if the expected type is an infer var. This causes that array doesn't support nested coercion. ```rust fn foo() {} fn bar() {} fn main() { let _ = [foo, if false { bar } else { foo }]; // type mismatch when trying to coerce `bar` into `foo` in if-then branch coercion. } ``` But we can't simply change this behavior to be the same as `if` and `match` since [many code](rust-lang#140283 (comment)) depends on using the first element's type as expectation.
2 parents 842bd5b + b235cc2 commit 35a31ba

File tree

5 files changed

+92
-1
lines changed

5 files changed

+92
-1
lines changed

compiler/rustc_hir_typeck/src/coercion.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1365,6 +1365,7 @@ pub fn can_coerce<'tcx>(
13651365
/// - WARNING: I don't believe this final type is guaranteed to be
13661366
/// related to your initial `expected_ty` in any particular way,
13671367
/// although it will typically be a subtype, so you should check it.
1368+
/// Check the note below for more details.
13681369
/// - Invoking `complete()` may cause us to go and adjust the "adjustments" on
13691370
/// previously coerced expressions.
13701371
///
@@ -1378,6 +1379,28 @@ pub fn can_coerce<'tcx>(
13781379
/// }
13791380
/// let final_ty = coerce.complete(fcx);
13801381
/// ```
1382+
///
1383+
/// NOTE: Why does the `expected_ty` participate in the LUB?
1384+
/// When coercing, each branch should use the following expectations for type inference:
1385+
/// - The branch can be coerced to the expected type of the match/if/whatever.
1386+
/// - The branch can be coercion lub'd with the types of the previous branches.
1387+
/// Ideally we'd have some sort of `Expectation::ParticipatesInCoerceLub(ongoing_lub_ty, final_ty)`,
1388+
/// but adding and using this feels very challenging.
1389+
/// What we instead do is to use the expected type of the match/if/whatever as
1390+
/// the initial coercion lub. This allows us to use the lub of "expected type of match" with
1391+
/// "types from previous branches" as the coercion target, which can contains both expectations.
1392+
///
1393+
/// Two concerns with this approach:
1394+
/// - We may have incompatible `final_ty` if that lub is different from the expected
1395+
/// type of the match. However, in this case coercing the final type of the
1396+
/// `CoerceMany` to its expected type would have error'd anyways, so we don't care.
1397+
/// - We may constrain the `expected_ty` too early. For some branches with
1398+
/// type `a` and `b`, we end up with `(a lub expected_ty) lub b` instead of
1399+
/// `(a lub b) lub expected_ty`. They should be the same type. However,
1400+
/// `a lub expected_ty` may constrain inference variables in `expected_ty`.
1401+
/// In this case the difference does matter and we get actually incorrect results.
1402+
/// FIXME: Ideally we'd compute the final type without unnecessarily constraining
1403+
/// the expected type of the match when computing the types of its branches.
13811404
pub(crate) struct CoerceMany<'tcx> {
13821405
expected_ty: Ty<'tcx>,
13831406
final_ty: Option<Ty<'tcx>>,

compiler/rustc_hir_typeck/src/expr.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1670,11 +1670,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16701670

16711671
let coerce_to = expected
16721672
.to_option(self)
1673-
.and_then(|uty| self.try_structurally_resolve_type(expr.span, uty).builtin_index())
1673+
.and_then(|uty| {
1674+
self.try_structurally_resolve_type(expr.span, uty)
1675+
.builtin_index()
1676+
// Avoid using the original type variable as the coerce_to type, as it may resolve
1677+
// during the first coercion instead of being the LUB type.
1678+
.filter(|t| !self.try_structurally_resolve_type(expr.span, *t).is_ty_var())
1679+
})
16741680
.unwrap_or_else(|| self.next_ty_var(expr.span));
16751681
let mut coerce = CoerceMany::with_capacity(coerce_to, args.len());
16761682

16771683
for e in args {
1684+
// FIXME: the element expectation should use
1685+
// `try_structurally_resolve_and_adjust_for_branches` just like in `if` and `match`.
1686+
// While that fixes nested coercion, it will break [some
1687+
// code like this](https://github.com/rust-lang/rust/pull/140283#issuecomment-2958776528).
1688+
// If we find a way to support recursive tuple coercion, this break can be avoided.
16781689
let e_ty = self.check_expr_with_hint(e, coerce_to);
16791690
let cause = self.misc(e.span);
16801691
coerce.coerce(self, &cause, e, e_ty);
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Weakened closure sig inference by #140283.
2+
fn foo<F: FnOnce(&str) -> usize, const N: usize>(x: [F; N]) {}
3+
4+
fn main() {
5+
foo([|s| s.len()])
6+
//~^ ERROR: type annotations needed
7+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error[E0282]: type annotations needed
2+
--> $DIR/closure-in-array.rs:5:11
3+
|
4+
LL | foo([|s| s.len()])
5+
| ^ - type must be known at this point
6+
|
7+
help: consider giving this closure parameter an explicit type
8+
|
9+
LL | foo([|s: /* Type */| s.len()])
10+
| ++++++++++++
11+
12+
error: aborting due to 1 previous error
13+
14+
For more information about this error, try `rustc --explain E0282`.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
//@ run-pass
2+
// Check that least upper bound coercions don't resolve type variable merely based on the first
3+
// coercion. Check issue #136420.
4+
5+
fn foo() {}
6+
fn bar() {}
7+
8+
fn infer<T>(_: T) {}
9+
10+
fn infer_array_element<T>(_: [T; 2]) {}
11+
12+
fn main() {
13+
// Previously the element type's ty var will be unified with `foo`.
14+
let _: [_; 2] = [foo, bar];
15+
infer_array_element([foo, bar]);
16+
17+
let _ = if false {
18+
foo
19+
} else {
20+
bar
21+
};
22+
infer(if false {
23+
foo
24+
} else {
25+
bar
26+
});
27+
28+
let _ = match false {
29+
true => foo,
30+
false => bar,
31+
};
32+
infer(match false {
33+
true => foo,
34+
false => bar,
35+
});
36+
}

0 commit comments

Comments
 (0)