fix accidental type variable resolution in array coercion

Author: adwinwhite

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.
 ///
@@ -1379,6 +1380,30 @@ pub fn can_coerce<'tcx>(
 /// let final_ty = coerce.complete(fcx);
 /// ```
 pub(crate) struct CoerceMany<'tcx> {
+///
+/// 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, 'exprs, E: AsCoercionSite> {
+>>>>>>> d04050504b7 (doc: explain why `expected_ty` participates in the LUB of `CoerceMany`)
     expected_ty: Ty<'tcx>,
     final_ty: Option<Ty<'tcx>>,
     expressions: Vec<&'tcx hir::Expr<'tcx>>,

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);

tests/ui/coercion/closure-in-array.rs

@@ -0,0 +1,7 @@
+// Weakened closure sig inference by #140283.
+fn foo<F: FnOnce(&str) -> usize, const N: usize>(x: [F; N]) {}
+
+fn main() {
+    foo([|s| s.len()])
+    //~^ ERROR: type annotations needed
+}

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`.

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>(_: T) {}
+
+fn infer_array_element<T>(_: [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,
+    });
+}