From b235cc22acdde9be7a25c3b5828e9c5863a0b54f Mon Sep 17 00:00:00 2001 From: Adwin White Date: Thu, 7 Aug 2025 19:41:57 +0800 Subject: [PATCH] fix accidental type variable resolution in array coercion --- compiler/rustc_hir_typeck/src/coercion.rs | 23 +++++++++++++ compiler/rustc_hir_typeck/src/expr.rs | 13 ++++++- tests/ui/coercion/closure-in-array.rs | 7 ++++ tests/ui/coercion/closure-in-array.stderr | 14 ++++++++ tests/ui/coercion/coerce-many-with-ty-var.rs | 36 ++++++++++++++++++++ 5 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/ui/coercion/closure-in-array.rs create mode 100644 tests/ui/coercion/closure-in-array.stderr create mode 100644 tests/ui/coercion/coerce-many-with-ty-var.rs diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 52ea6cdeaa0eb..0e87b90dc6c11 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1365,6 +1365,7 @@ pub fn can_coerce<'tcx>( /// - WARNING: I don't believe this final type is guaranteed to be /// related to your initial `expected_ty` in any particular way, /// although it will typically be a subtype, so you should check it. +/// Check the note below for more details. /// - Invoking `complete()` may cause us to go and adjust the "adjustments" on /// previously coerced expressions. /// @@ -1378,6 +1379,28 @@ pub fn can_coerce<'tcx>( /// } /// let final_ty = coerce.complete(fcx); /// ``` +/// +/// NOTE: Why does the `expected_ty` participate in the LUB? +/// When coercing, each branch should use the following expectations for type inference: +/// - The branch can be coerced to the expected type of the match/if/whatever. +/// - The branch can be coercion lub'd with the types of the previous branches. +/// Ideally we'd have some sort of `Expectation::ParticipatesInCoerceLub(ongoing_lub_ty, final_ty)`, +/// but adding and using this feels very challenging. +/// What we instead do is to use the expected type of the match/if/whatever as +/// the initial coercion lub. This allows us to use the lub of "expected type of match" with +/// "types from previous branches" as the coercion target, which can contains both expectations. +/// +/// Two concerns with this approach: +/// - We may have incompatible `final_ty` if that lub is different from the expected +/// type of the match. However, in this case coercing the final type of the +/// `CoerceMany` to its expected type would have error'd anyways, so we don't care. +/// - We may constrain the `expected_ty` too early. For some branches with +/// type `a` and `b`, we end up with `(a lub expected_ty) lub b` instead of +/// `(a lub b) lub expected_ty`. They should be the same type. However, +/// `a lub expected_ty` may constrain inference variables in `expected_ty`. +/// In this case the difference does matter and we get actually incorrect results. +/// FIXME: Ideally we'd compute the final type without unnecessarily constraining +/// the expected type of the match when computing the types of its branches. pub(crate) struct CoerceMany<'tcx> { expected_ty: Ty<'tcx>, final_ty: Option>, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 885af3b909b8f..5b40531f94627 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1670,11 +1670,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let coerce_to = expected .to_option(self) - .and_then(|uty| self.try_structurally_resolve_type(expr.span, uty).builtin_index()) + .and_then(|uty| { + self.try_structurally_resolve_type(expr.span, uty) + .builtin_index() + // Avoid using the original type variable as the coerce_to type, as it may resolve + // during the first coercion instead of being the LUB type. + .filter(|t| !self.try_structurally_resolve_type(expr.span, *t).is_ty_var()) + }) .unwrap_or_else(|| self.next_ty_var(expr.span)); let mut coerce = CoerceMany::with_capacity(coerce_to, args.len()); for e in args { + // FIXME: the element expectation should use + // `try_structurally_resolve_and_adjust_for_branches` just like in `if` and `match`. + // While that fixes nested coercion, it will break [some + // code like this](https://github.com/rust-lang/rust/pull/140283#issuecomment-2958776528). + // If we find a way to support recursive tuple coercion, this break can be avoided. let e_ty = self.check_expr_with_hint(e, coerce_to); let cause = self.misc(e.span); coerce.coerce(self, &cause, e, e_ty); diff --git a/tests/ui/coercion/closure-in-array.rs b/tests/ui/coercion/closure-in-array.rs new file mode 100644 index 0000000000000..ca5c021c77a7f --- /dev/null +++ b/tests/ui/coercion/closure-in-array.rs @@ -0,0 +1,7 @@ +// Weakened closure sig inference by #140283. +fn foo usize, const N: usize>(x: [F; N]) {} + +fn main() { + foo([|s| s.len()]) + //~^ ERROR: type annotations needed +} diff --git a/tests/ui/coercion/closure-in-array.stderr b/tests/ui/coercion/closure-in-array.stderr new file mode 100644 index 0000000000000..90cf590c09e73 --- /dev/null +++ b/tests/ui/coercion/closure-in-array.stderr @@ -0,0 +1,14 @@ +error[E0282]: type annotations needed + --> $DIR/closure-in-array.rs:5:11 + | +LL | foo([|s| s.len()]) + | ^ - type must be known at this point + | +help: consider giving this closure parameter an explicit type + | +LL | foo([|s: /* Type */| s.len()]) + | ++++++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/tests/ui/coercion/coerce-many-with-ty-var.rs b/tests/ui/coercion/coerce-many-with-ty-var.rs new file mode 100644 index 0000000000000..cbd16f58ea5b5 --- /dev/null +++ b/tests/ui/coercion/coerce-many-with-ty-var.rs @@ -0,0 +1,36 @@ +//@ run-pass +// Check that least upper bound coercions don't resolve type variable merely based on the first +// coercion. Check issue #136420. + +fn foo() {} +fn bar() {} + +fn infer(_: T) {} + +fn infer_array_element(_: [T; 2]) {} + +fn main() { + // Previously the element type's ty var will be unified with `foo`. + let _: [_; 2] = [foo, bar]; + infer_array_element([foo, bar]); + + let _ = if false { + foo + } else { + bar + }; + infer(if false { + foo + } else { + bar + }); + + let _ = match false { + true => foo, + false => bar, + }; + infer(match false { + true => foo, + false => bar, + }); +}