Skip to content

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Jul 25, 2024

Implements a similar check to the one that we have in projection for GATs (#102488, #123240), where we check that the args of an impl item are compatible before returning it. This is done in resolve_assoc_item, which is backing Instance::resolve, so this is conceptually generalizing the check from GATs to methods/assoc consts. This is important to make sure that the inliner will only visit and substitute MIR bodies that are compatible w/ their trait definitions.

This shouldn't happen in codegen, but there are a few ways to get the inliner to be invoked (via calls to optimized_mir) before codegen, namely polymorphization and CTFE.

Fixes #121957
Fixes #120792
Fixes #120793
Fixes #121063

@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2024

r? @cjgillot

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

@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. labels Jul 25, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jul 25, 2024

r? @oli-obk

bors r plus

@bors
Copy link
Collaborator

bors commented Jul 25, 2024

📌 Commit 40d132f has been approved by oli-obk

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 Jul 25, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#121364 (Implement lint against ambiguous negative literals)
 - rust-lang#127300 (Fix connect timeout for non-linux targets, read readiness of socket connection, Read readiness to detect errors. `Fixes rust-lang#127018`)
 - rust-lang#128138 (`#[naked]`: use an allowlist for allowed options on `asm!` in naked functions)
 - rust-lang#128158 (std: unsafe-wrap personality::gcc)
 - rust-lang#128171 (Make sure that args are compatible in `resolve_associated_item`)
 - rust-lang#128172 (Don't ICE if HIR and middle types disagree in borrowck error reporting)
 - rust-lang#128173 (Remove crashes for misuses of intrinsics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5a853d0 into rust-lang:master Jul 25, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Jul 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2024
Rollup merge of rust-lang#128171 - compiler-errors:arg-compat, r=oli-obk

Make sure that args are compatible in `resolve_associated_item`

Implements a similar check to the one that we have in projection for GATs (rust-lang#102488, rust-lang#123240), where we check that the args of an impl item are compatible before returning it. This is done in `resolve_assoc_item`, which is backing `Instance::resolve`, so this is conceptually generalizing the check from GATs to methods/assoc consts. This is important to make sure that the inliner will only visit and substitute MIR bodies that are compatible w/ their trait definitions.

This shouldn't happen in codegen, but there are a few ways to get the inliner to be invoked (via calls to `optimized_mir`) before codegen, namely polymorphization and CTFE.

Fixes rust-lang#121957
Fixes rust-lang#120792
Fixes rust-lang#120793
Fixes rust-lang#121063
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 8, 2024
…ir, r=davidtwco

Make `validate_mir` ensure the final MIR for all bodies

A lot of the crashes tests use `-Zpolymorphize` or `-Zdump-mir` for their side effect of computing the `optimized_mir` for all bodies, which will uncover bugs with late MIR passes like the inliner. I don't like having all these tests depend on `-Zpolymorphize` (or other hacky ways) for no reason, so this PR extends the `-Zvalidate-mir` flag to ensure `optimized_mir`/`mir_for_ctfe` for all body owners during the analysis phase.

Two thoughts:
1. This could be moved later in the compilation pipeline I guess? I don't really think it matters, though.
1. This could alternatively be expressed using a new flag, though I don't necessarily see much value in separating these.

For example, rust-lang#128171 could have used this flag, in the `tests/ui/polymorphization/inline-incorrect-early-bound.rs`.

r? mir
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 8, 2024
…ir, r=davidtwco

Make `validate_mir` ensure the final MIR for all bodies

A lot of the crashes tests use `-Zpolymorphize` or `-Zdump-mir` for their side effect of computing the `optimized_mir` for all bodies, which will uncover bugs with late MIR passes like the inliner. I don't like having all these tests depend on `-Zpolymorphize` (or other hacky ways) for no reason, so this PR extends the `-Zvalidate-mir` flag to ensure `optimized_mir`/`mir_for_ctfe` for all body owners during the analysis phase.

Two thoughts:
1. This could be moved later in the compilation pipeline I guess? I don't really think it matters, though.
1. This could alternatively be expressed using a new flag, though I don't necessarily see much value in separating these.

For example, rust-lang#128171 could have used this flag, in the `tests/ui/polymorphization/inline-incorrect-early-bound.rs`.

r? mir
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2024
Rollup merge of rust-lang#128612 - compiler-errors:validate-mir-opt-mir, r=davidtwco

Make `validate_mir` ensure the final MIR for all bodies

A lot of the crashes tests use `-Zpolymorphize` or `-Zdump-mir` for their side effect of computing the `optimized_mir` for all bodies, which will uncover bugs with late MIR passes like the inliner. I don't like having all these tests depend on `-Zpolymorphize` (or other hacky ways) for no reason, so this PR extends the `-Zvalidate-mir` flag to ensure `optimized_mir`/`mir_for_ctfe` for all body owners during the analysis phase.

Two thoughts:
1. This could be moved later in the compilation pipeline I guess? I don't really think it matters, though.
1. This could alternatively be expressed using a new flag, though I don't necessarily see much value in separating these.

For example, rust-lang#128171 could have used this flag, in the `tests/ui/polymorphization/inline-incorrect-early-bound.rs`.

r? mir
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 9, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 9, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2025
Rollup merge of rust-lang#140819 - reddevilmidzy:add-test, r=petrochenkov

Add regression test for 125877

close: rust-lang#125877

rust-lang#125877 (comment)  has been resolved rust-lang#128171
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.
Projects
None yet
5 participants