rust Use equality when relating formal and expected type in arg checking

#129059 uncovered an interesting issue in argument checking. When we check arguments, we create three sets of types:

Formals
Expected
Actuals

The actuals are the types of the argument expressions themselves. The formals are the types from the signature that we're checking. The expected types are the formal types, but passed through expected_inputs_for_expected_outputs:

https://github.com/rust-lang/rust/blob/a971212545766fdfe0dd68e5d968133f79944a19/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs#L691-L725

This method attempts to constrain the formal inputs by relating the expectation of the call expression and the formal output.

When we check an argument, we get the expression's actual type, and then we first attempt to coerce it to the expected type:

https://github.com/rust-lang/rust/blob/a971212545766fdfe0dd68e5d968133f79944a19/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L280-L293

Then we subtype the expected type and the formal type:

https://github.com/rust-lang/rust/blob/a971212545766fdfe0dd68e5d968133f79944a19/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs#L299-L305

However, since we are now recording the right coercion target (since #129059), we now end up recording the expected type to the typeck results, rather than the actual.

Since that expected type was fudged, it has fresh variables. And since the expected type is only subtyped against the formal type, if that expected type has a bivariant parameter, it will likely remain unconstrained since Covariant * Bivariant = Bivariant according to xform. This leads to an unconstrained type variable in writeback.

AFAICT, there's no reason for us to be using subtyping here, though. The expected output is already related to the expectation by subtyping:

https://github.com/rust-lang/rust/blob/a971212545766fdfe0dd68e5d968133f79944a19/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs#L713

So the formals don't need "another" indirection of subtyping in the argument checking... So I've changed it to use equality here. We could alternatively fix this by requiring WF for all the expected types to constrain their bivariant parameters, but this seems a bit overkill.

Fixes #129286

Aug 20 '24 15:08 compiler-errors

r? @jieyouxu

rustbot has assigned @jieyouxu. 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

Aug 20 '24 15:08 rustbot

I expect the only person who's able to review this to be lcnr, who is gone, but I guess I'll just assign it to them.

r? lcnr

Aug 20 '24 15:08 compiler-errors

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

------
 > importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:20d3b4d4a2629cbf7865cdbf92fe47512a7c96658c24253a045ff38e8075cd7fb37ca6fcadfa6e6d093333943ad24f6fc4f163ec5b74fd940de9d5bb03eb4d3b:
------
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
---- [ui] tests/ui/coercion/constrain-expectation-in-arg.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/coercion/constrain-expectation-in-arg.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/coercion/constrain-expectation-in-arg" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/coercion/constrain-expectation-in-arg/auxiliary"
--- stderr -------------------------------
error[E0601]: `main` function not found in crate `constrain_expectation_in_arg`
##[error]  --> /checkout/tests/ui/coercion/constrain-expectation-in-arg.rs:17:2
   |

Aug 20 '24 16:08 rust-log-analyzer

@bors try @rust-timer queue

Aug 25 '24 15:08 compiler-errors

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

Aug 25 '24 15:08 rust-timer

:hourglass: Trying commit c2863260b31011c67dfebc6acdb11b7b18591e02 with merge fc833508d4035a6f865a324d0f570136e4fa4081...

Aug 25 '24 15:08 bors

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#16 2.673 Building wheels for collected packages: reuse
#16 2.674   Building wheel for reuse (pyproject.toml): started
#16 2.917   Building wheel for reuse (pyproject.toml): finished with status 'done'
#16 2.918   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#16 2.918   Stored in directory: /tmp/pip-ephem-wheel-cache-hn_wc0ws/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#16 2.921 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#16 3.315 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#16 3.316 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#16 DONE 3.4s
---
   |
39 |     self, NormalizeExt, ObligationCauseCode, ObligationCtxt, StructurallyNormalizeExt,
   |                                              ^^^^^^^^^^^^^^

error[E0599]: no method named `expected_inputs_for_expected_output` found for reference `&fn_ctxt::FnCtxt<'a, 'tcx>` in the current scope
     |
     |
1672 |             self.expected_inputs_for_expected_output(span, expected, adt_ty, &[adt_ty]);
     |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `&FnCtxt<'a, 'tcx>`
For more information about this error, try `rustc --explain E0599`.
error: could not compile `rustc_hir_typeck` (lib) due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `rustc_hir_typeck` (lib test) due to 3 previous errors

Aug 25 '24 15:08 rust-log-analyzer

The job dist-x86_64-linux failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

file:.git/config remote.origin.url=https://github.com/rust-lang-ci/rust
file:.git/config remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
file:.git/config gc.auto=0
file:.git/config http.https://github.com/.extraheader=AUTHORIZATION: basic ***
file:.git/config branch.try.remote=origin
file:.git/config branch.try.merge=refs/heads/try
file:.git/config submodule.library/backtrace.url=https://github.com/rust-lang/backtrace-rs.git
file:.git/config submodule.library/stdarch.active=true
file:.git/config submodule.library/stdarch.url=https://github.com/rust-lang/stdarch.git
file:.git/config submodule.src/doc/book.active=true
---
   |
39 |     self, NormalizeExt, ObligationCauseCode, ObligationCtxt, StructurallyNormalizeExt,
   |                                              ^^^^^^^^^^^^^^

error[E0599]: no method named `expected_inputs_for_expected_output` found for reference `&fn_ctxt::FnCtxt<'a, 'tcx>` in the current scope
     |
     |
1672 |             self.expected_inputs_for_expected_output(span, expected, adt_ty, &[adt_ty]);
     |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method not found in `&FnCtxt<'a, 'tcx>`
For more information about this error, try `rustc --explain E0599`.
[RUSTC-TIMING] rustc_hir_typeck test:false 6.699
error: could not compile `rustc_hir_typeck` (lib) due to 3 previous errors
warning: build failed, waiting for other jobs to finish...
---
Caused by:
    Command RUST_BACKTRACE=full python3 /checkout/x.py build --target x86_64-unknown-linux-gnu --host x86_64-unknown-linux-gnu --stage 2 library/std --rust-profile-generate /tmp/tmp-multistage/opt-artifacts/rustc-pgo --set llvm.thin-lto=false --set llvm.link-shared=true [at /checkout/obj] has failed with exit code Some(1)

Stack backtrace:
   0: <anyhow::Error>::msg::<alloc::string::String>
             at /rust/deps/anyhow-1.0.86/src/backtrace.rs:27:14
   1: <opt_dist::exec::CmdBuilder>::run
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/exec.rs:80:17
   2: <opt_dist::exec::Bootstrap>::run
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/exec.rs:181:9
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/main.rs:225:13
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/main.rs:225:13
   4: <opt_dist::timer::TimerSection>::section::<opt_dist::execute_pipeline::{closure#1}::{closure#0}, ()>
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/timer.rs:111:22
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/main.rs:214:9
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/main.rs:214:9
   6: <opt_dist::timer::TimerSection>::section::<opt_dist::execute_pipeline::{closure#1}, opt_dist::training::RustcPGOProfile>
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/timer.rs:111:22
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/main.rs:211:29
   8: opt_dist::main
             at /rustc/fc833508d4035a6f865a324d0f570136e4fa4081/src/tools/opt-dist/src/main.rs:401:18
   9: <fn() -> core::result::Result<(), anyhow::Error> as core::ops::function::FnOnce<()>>::call_once
   9: <fn() -> core::result::Result<(), anyhow::Error> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/08328a323ecd80b443a8fcc72c6b1071f48e233f/library/core/src/ops/function.rs:250:5
  10: std::sys::backtrace::__rust_begin_short_backtrace::<fn() -> core::result::Result<(), anyhow::Error>, core::result::Result<(), anyhow::Error>>
             at /rustc/08328a323ecd80b443a8fcc72c6b1071f48e233f/library/std/src/sys/backtrace.rs:152:18
  11: std::rt::lang_start::<core::result::Result<(), anyhow::Error>>::{closure#0}
             at /rustc/08328a323ecd80b443a8fcc72c6b1071f48e233f/library/std/src/rt.rs:162:18
  12: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once
  13: std::panicking::try::do_call
             at /rustc/08328a323ecd80b443a8fcc72c6b1071f48e233f/library/std/src/panicking.rs:557:40
  14: std::panicking::try
             at /rustc/08328a323ecd80b443a8fcc72c6b1071f48e233f/library/std/src/panicking.rs:521:19

Aug 25 '24 16:08 rust-log-analyzer

:broken_heart: Test failed - checks-actions

Aug 25 '24 16:08 bors

@bors try @rust-timer queue

Aug 25 '24 16:08 compiler-errors

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

Aug 25 '24 16:08 rust-timer

:hourglass: Trying commit f6f5d72ab2dfd794b7fc4f61d79ea1935bfaaf71 with merge b4cbff954774fa306ee6a05617024c2e1484732d...

Aug 25 '24 16:08 bors

:sunny: Try build successful - checks-actions Build commit: b4cbff954774fa306ee6a05617024c2e1484732d (b4cbff954774fa306ee6a05617024c2e1484732d)

Aug 25 '24 18:08 bors

Queued b4cbff954774fa306ee6a05617024c2e1484732d with parent 89103466d77a3ae068bab0fd17c53bf7104f627b, future comparison URL. There is currently 1 preceding artifact in the queue. It will probably take at least ~2.0 hours until the benchmark run finishes.

Aug 25 '24 18:08 rust-timer

Finished benchmarking commit (b4cbff954774fa306ee6a05617024c2e1484732d): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never @rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	7
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.4%, secondary -1.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-3.2%, -2.2%]	2
All ❌✅ (primary)	2.4%	[2.4%, 2.4%]	1

Cycles

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

Binary size

Results (secondary -0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.0%	[-0.0%, -0.0%]	1
All ❌✅ (primary)	-	-	0

Bootstrap: 751.721s -> 751.87s (0.02%) Artifact size: 338.75 MiB -> 338.82 MiB (0.02%)

Aug 25 '24 20:08 rust-timer

probably not worth that last commit just because it does very little

Aug 25 '24 21:08 compiler-errors

@rustbot ready

I don't think any more changes can be done, and if they should, I'd rather do them as follow-ups.

Aug 27 '24 11:08 compiler-errors

@bors r+

Aug 29 '24 10:08 lcnr

:pushpin: Commit 3a6b3df83717d9d3020c6048f7cde35309a2076a has been approved by lcnr

It is now in the queue for this repository.

Aug 29 '24 10:08 bors

:umbrella: The latest upstream changes (presumably #129817) made this pull request unmergeable. Please resolve the merge conflicts.

Aug 31 '24 20:08 bors

@bors r=lcnr

Aug 31 '24 20:08 compiler-errors

:pushpin: Commit 95b9ecd6d671637e9e3db55ed31d06882d3cad4d has been approved by lcnr

It is now in the queue for this repository.

Aug 31 '24 20:08 bors

:hourglass: Testing commit 95b9ecd6d671637e9e3db55ed31d06882d3cad4d with merge 3cb42da3fa8dc5c2a558ccca1b0374a986f9193f...

Sep 02 '24 12:09 bors

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[RUSTC-TIMING] miri test:false 4.518
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Mon, Sep  2, 2024  1:39:36 PM
  network time: Mon, 02 Sep 2024 13:39:36 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

Sep 02 '24 13:09 rust-log-analyzer

:broken_heart: Test failed - checks-actions

Sep 02 '24 13:09 bors

@bors retry

Sep 02 '24 13:09 compiler-errors

:hourglass: Testing commit 95b9ecd6d671637e9e3db55ed31d06882d3cad4d with merge bd53aa3bf7a24a70d763182303bd75e5fc51a9af...

Sep 02 '24 16:09 bors

:sunny: Test successful - checks-actions Approved by: lcnr Pushing bd53aa3bf7a24a70d763182303bd75e5fc51a9af to master...

Sep 02 '24 18:09 bors

Finished benchmarking commit (bd53aa3bf7a24a70d763182303bd75e5fc51a9af): 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)

Results (secondary -2.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-2.7%, -2.7%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 750.422s -> 750.044s (-0.05%) Artifact size: 338.30 MiB -> 338.29 MiB (-0.00%)

Sep 02 '24 19:09 rust-timer

@rustbot label +beta-nominated

Fixes a beta regression https://github.com/rust-lang/rust/issues/130576 which was introduced by https://github.com/rust-lang/rust/pull/129059, but which i only caught in the crater run for https://github.com/rust-lang/rust/pull/129021#issuecomment-2297702647

Sep 19 '24 20:09 compiler-errors

rust rust copied to clipboard

Use equality when relating formal and expected type in arg checking

Overall result: ✅ improvements - no action needed

Overall result: no relevant changes - no action needed

rust
rust copied to clipboard