Fix accidental type inference in array coercion

[adwinwhite]

Fixes #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 and check_expr_if where CoerceMany is also used do the same.

FCP Proposal:

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

fn foo() {}
fn bar() {} 
fn f<T>(_: [T; 2]) {}

f([foo, bar]);

and

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(#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.

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 depends on using the first element's type as expectation.

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jieyouxu]

Please add a test to demonstrate the effect of this change. Especially since with this change, we would be accepting more code than on master (this is the snippet in the original issue):

fn foo() {}
fn bar() {}

fn main() {
    let _a = if true { foo } else { bar };
    let _b = vec![foo, bar];
    let _c = [foo, bar];
    d(if true { foo } else { bar });
    e(vec![foo, bar]);
    f([foo, bar]);  // <- this PR now accepts this
}

fn d<T>(_: T) {}
fn e<T>(_: Vec<T>) {}
fn f<T>(_: [T; 2]) {}

whereas on master this snippet does not compile w/

error[E0308]: mismatched types
  --> src/main.rs:10:7
   |
10 |     f([foo, bar]);
   |     - ^^^^^^^^^^ expected `[fn() {foo}; 2]`, found `[fn(); 2]`
   |     |
   |     arguments to this function are incorrect
   |
   = note: expected array `[fn() {foo}; 2]`
              found array `[fn(); 2]`
note: function defined here
  --> src/main.rs:15:4
   |
15 | fn f<T>(_: [T; 2]) {}
   |    ^    ---------

For more information about this error, try `rustc --explain E0308`.

I'm surprised there are no ui test diffs.

[petrochenkov]

r? types

[adwinwhite]

There're some similar errors, but I'm unsure whether it's okay to allow these code. The Rust Reference.

fn foo() {}
fn bar() {}

fn main() {
    let block_var = 'a: { // Easy to fix, but not specified by the Rust Reference.
        if false {
            break 'a foo;
        }
        break 'a bar;
    };

    let loop_var = loop {  // Easy to fix, but not specified by the Rust Reference.
        if false {
            break foo;
        }
        break bar;
    };

    let closure_var = || { // More complicated. But this should work according to the Rust Reference.
        if false {
            return foo;
        }
        return bar;
    };
}

[oli-obk]

There're some similar errors, but I'm unsure whether it's okay to allow these code

Yea I think these all should work, too. Please fix the easy ones and add tests for all of them if we don't already have any

[adwinwhite]

Turns out the ‘easy’ cases aren’t so easy after all. I can't fix the inference regressions for now, but I might revisit them later once I understand type inference better.

[adwinwhite]

@rustbot ready

[oli-obk]

I am not going to get to this in the next 10 days. I'll need to review the general state of inference here and write a T-types FCP text

[bors]

☔ The latest upstream changes (presumably #141716) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

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

fn foo() {}
fn bar() {} 
fn f<T>(_: [T; 2]) {}

f([foo, bar]);

and

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

@rfcbot merge

[rfcbot]

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @oli-obk
• @spastorino

Concerns:

• try going all the way, no expectation resolved by Fix accidental type inference in array coercion #140283 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[BoxyUwU]

I think that was previously tried upthread somewhere and it broke a lot of stuff, i can't remember where though. but note that other places in inference that use CoerceMany already have such an issue. e.g. if else has weird behaviour around expectations with inference vars

[jackh726]

I think that was previously tried upthread somewhere and it broke a lot of stuff

I don't think the specific variant I am suggesting (checking if coerce_to contains any inference variable) has been checked, only with the try_structurally_resolve_and_adjust_for_branches call, which removes any expectation if coerce_to is an infer var. This is both stronger (because of NoExpectation but also weaker because of is_ty_var()`).

[adwinwhite]

I'm realizing, that I think this PR is insufficient to prevent all array coercion from the first element. You can have a coerce_to that contains an inference variables, but is not a var itself.

You're right that it's insufficient. I think it's not observable now. Since coercion only works for the outmost type, the code will fail lub coercion anyway.
Nonetheless, I'll change them to use has_infer_vars in case lub coercion is enhanced in the future.

But the coercion might need the expectation type to make progress even if it contains ty vars.
It seems too strict to replace the expectation with a new infer var in that case.
Let me test the changes locally first.

[adwinwhite]

new branch diff

    let _: Vec<Box<dyn Fn(isize) -> _>> = vec![
        Box::new(|x| (x as u8)),
        Box::new(|x| (x as i16 as u8)),
    ];

This fails with log:

error[E0308]: mismatched types
  --> /home/adwin/Code/rust/rust/base/tests/ui/coercion/coerce-expect-unsized.rs:39:43
   |
LL |       let _: Vec<Box<dyn Fn(isize) -> _>> = vec![
   |  ___________________________________________^
LL | |         Box::new(|x| (x as u8)),
   | |                  --- the found closure
LL | |         Box::new(|x| (x as i16 as u8)),
LL | |     ];
   | |     ^
   | |     |
   | |_____expected `Box<[Box<dyn Fn(isize) -> _>]>`, found `Box<[Box<{closure@...}>; 2]>`
   |       arguments to this function are incorrect

[jackh726]

Ah, yep. That one. I ran across that one too, and I think was the one that made me just not change array coercion in my testing (370fc9b#diff-502fc8fe28ac40b5ad34931ece468fa6755533b1aeb4ce89c528718ec9cef106R1684), because I don't actually know how to solve that.

[jackh726]

Alright, so, I'm good to land the PR as-is, and it's been FCPed. Can you rebase?

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[jackh726]

I see the date that this was opened and am terribly sorry that you've waited this long @adwinwhite. That being said, I appreciate you opening this and seeing it through :)

Overall here, even though I am doing other experimentation in this area, these changes here align with the principles that I am trying to achieve (that the LUB inference is "somewhat" independent from the "expected type" - and also doesn't directly result in any additional order-dependent behavior).

@bors r+ rollup=never

[rust-bors]

📌 Commit b235cc2 has been approved by jackh726

It is now in the queue for this repository.

[rust-bors]

☀️ Test successful - CI
Approved by: jackh726
Duration: 3h 7m 24s
Pushing 35a31ba to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 842bd5b (parent) -> 35a31ba (this PR)

Test differences

Show 14 test diffs

Stage 1

• [ui] tests/ui/coercion/closure-in-array.rs: [missing] -> pass (J1)
• [ui] tests/ui/coercion/coerce-many-with-ty-var.rs: [missing] -> pass (J1)

Stage 2

• [ui] tests/ui/coercion/closure-in-array.rs: [missing] -> pass (J0)
• [ui] tests/ui/coercion/coerce-many-with-ty-var.rs: [missing] -> pass (J0)

Additionally, 10 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: aarch64-apple, aarch64-gnu, aarch64-gnu-llvm-20-1, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-gnu, x86_64-gnu-debug, x86_64-gnu-gcc, x86_64-gnu-llvm-20, x86_64-gnu-llvm-20-2, x86_64-gnu-llvm-21-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
• J1: x86_64-gnu-llvm-20-3, x86_64-gnu-llvm-21-3

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 35a31ba763976907cd38ba88743a3aefbaf8ffff --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. aarch64-apple: 12512.1s -> 8964.1s (-28.4%)
2. x86_64-gnu-gcc: 4165.1s -> 3407.2s (-18.2%)
3. pr-check-1: 1942.5s -> 1611.1s (-17.1%)
4. dist-aarch64-apple: 7968.9s -> 6754.1s (-15.2%)
5. x86_64-gnu-tools: 3801.2s -> 3304.8s (-13.1%)
6. i686-gnu-2: 6028.9s -> 5251.1s (-12.9%)
7. x86_64-gnu-miri: 4789.1s -> 4172.3s (-12.9%)
8. x86_64-rust-for-linux: 3098.3s -> 2704.6s (-12.7%)
9. aarch64-gnu-llvm-20-1: 3875.2s -> 3427.0s (-11.6%)
10. aarch64-msvc-2: 5890.6s -> 6546.6s (+11.1%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (35a31ba): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.781s -> 476.214s (0.51%)
Artifact size: 397.87 MiB -> 397.88 MiB (0.00%)