Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record the correct target type when coercing fn items/closures to pointers #129059

Merged
merged 2 commits into from
Aug 14, 2024

Conversation

compiler-errors
Copy link
Member

Self-explanatory. We were previously not recording the target type of a coercion as the output of an adjustment. This should remedy that.

We must also modify the function pointer casts in MIR typeck to use subtyping, since those broke since #118247.

r? lcnr

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 13, 2024
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a test which requires the sub for closures, is case nothing hit that before?

tests/mir-opt/build_correct_coerce.rs Show resolved Hide resolved
@compiler-errors compiler-errors changed the title Record the correct target type when coercing fn items to pointers Record the correct target type when coercing fn items/closures to pointers Aug 13, 2024
@lcnr
Copy link
Contributor

lcnr commented Aug 14, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 14, 2024

📌 Commit 5df13af has been approved by lcnr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 14, 2024
…type, r=lcnr

Record the correct target type when coercing fn items/closures to pointers

Self-explanatory. We were previously not recording the *target* type of a coercion as the output of an adjustment. This should remedy that.

We must also modify the function pointer casts in MIR typeck to use subtyping, since those broke since rust-lang#118247.

r? lcnr
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#128828 (`-Znext-solver` caching)
 - rust-lang#128954 (Explicitly specify type parameter on FromResidual for Option and ControlFlow.)
 - rust-lang#129054 (Subtree update of `rust-analyzer`)
 - rust-lang#129059 (Record the correct target type when coercing fn items/closures to pointers)
 - rust-lang#129071 (Port `run-make/sysroot-crates-are-unstable` to rmake)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2024
Rollup of 6 pull requests

Successful merges:

 - rust-lang#128570 (Stabilize `asm_const`)
 - rust-lang#128828 (`-Znext-solver` caching)
 - rust-lang#128954 (Explicitly specify type parameter on FromResidual for Option and ControlFlow.)
 - rust-lang#129059 (Record the correct target type when coercing fn items/closures to pointers)
 - rust-lang#129071 (Port `run-make/sysroot-crates-are-unstable` to rmake)
 - rust-lang#129088 (Make the rendered html doc for rustc better)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2200910 into rust-lang:master Aug 14, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2024
Rollup merge of rust-lang#129059 - compiler-errors:subtyping-correct-type, r=lcnr

Record the correct target type when coercing fn items/closures to pointers

Self-explanatory. We were previously not recording the *target* type of a coercion as the output of an adjustment. This should remedy that.

We must also modify the function pointer casts in MIR typeck to use subtyping, since those broke since rust-lang#118247.

r? lcnr
@compiler-errors
Copy link
Member Author

What the heck -- this PR caused two regressions:

[INFO] [stdout] error[E0308]: mismatched types
[INFO] [stdout]   --> src/lib.rs:30:4
[INFO] [stdout]    |
[INFO] [stdout] 30 | fn create_press_any_hook(ctx: &mut InlineCtx) {
[INFO] [stdout]    |    ^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
[INFO] [stdout]    |
[INFO] [stdout]    = note: expected fn pointer `unsafe extern "C" fn(&mut InlineCtx)`
[INFO] [stdout]               found fn pointer `unsafe extern "C" for<'a> fn(&'a mut InlineCtx)`
[INFO] [stdout] error[E0282]: type annotations needed
[INFO] [stdout]   --> src/blocking/server.rs:98:8
[INFO] [stdout]    |
[INFO] [stdout] 45 | #[derive(Clone, Debug)]
[INFO] [stdout]    |          ----- in this derive macro expansion
[INFO] [stdout] ...
[INFO] [stdout] 98 |   Send(Addrd<platform::Message<P>>),
[INFO] [stdout]    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for reference `&net::Addrd<kwap_msg::Message<<P as Platform>::MessagePayload, _, <P as Platform>::MessageOptions>>`
[INFO] [stdout]    |
[INFO] [stdout]    = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

I'll look into both of them, and revert this if not.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 25, 2024
…g, r=<try>

Use equality when relating formal and expected type in arg checking

rust-lang#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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_typeck/expectation/enum.Expectation.html) 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 rust-lang#129059), we now end up recording the expected type to the typeck results, rather than the actual.

Since that expected type was [fudged](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/struct.InferCtxt.html#method.fudge_inference_if_ok), 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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.Variance.html#method.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 rust-lang#129286
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 25, 2024
…g, r=<try>

Use equality when relating formal and expected type in arg checking

rust-lang#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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_typeck/expectation/enum.Expectation.html) 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 rust-lang#129059), we now end up recording the expected type to the typeck results, rather than the actual.

Since that expected type was [fudged](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/struct.InferCtxt.html#method.fudge_inference_if_ok), 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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.Variance.html#method.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 rust-lang#129286
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 25, 2024
…, r=lcnr

Use subtyping for `UnsafeFnPointer` coercion, too

I overlooked this in rust-lang#129059, which changed MIR typechecking to use subtyping for other fn pointer coercions.

Fixes rust-lang#129285
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 25, 2024
…, r=lcnr

Use subtyping for `UnsafeFnPointer` coercion, too

I overlooked this in rust-lang#129059, which changed MIR typechecking to use subtyping for other fn pointer coercions.

Fixes rust-lang#129285
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 26, 2024
Rollup merge of rust-lang#129288 - compiler-errors:unsafe-fn-coercion, r=lcnr

Use subtyping for `UnsafeFnPointer` coercion, too

I overlooked this in rust-lang#129059, which changed MIR typechecking to use subtyping for other fn pointer coercions.

Fixes rust-lang#129285
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 2, 2024
…g, r=lcnr

Use equality when relating formal and expected type in arg checking

rust-lang#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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_typeck/expectation/enum.Expectation.html) 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 rust-lang#129059), we now end up recording the expected type to the typeck results, rather than the actual.

Since that expected type was [fudged](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/struct.InferCtxt.html#method.fudge_inference_if_ok), 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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.Variance.html#method.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 rust-lang#129286
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 2, 2024
…g, r=lcnr

Use equality when relating formal and expected type in arg checking

rust-lang#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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_typeck/expectation/enum.Expectation.html) 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 rust-lang#129059), we now end up recording the expected type to the typeck results, rather than the actual.

Since that expected type was [fudged](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/struct.InferCtxt.html#method.fudge_inference_if_ok), 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](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/enum.Variance.html#method.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 rust-lang#129286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants