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

Add method to get type of an Rvalue in StableMIR #118688

Merged
merged 4 commits into from Dec 8, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Dec 6, 2023

Provide a method to StableMIR users to retrieve the type of an Rvalue operation. There were two possible implementation:

  1. Create the logic inside stable_mir to process the type according to the Rvalue semantics, which duplicates the logic of rustc_middle::mir::Rvalue::ty().
  2. Implement the Rvalue translation from StableMIR back to internal representation, invoke the rustc_middle::mir::Rvalue::ty(), and translate the return value to StableMIR.

I chose the first one for now since the duplication was fairly small, and the option 2 would require way more work to translate everything back to rustc internal representation. If we eventually add those translations, we could easily swap to the option 2.

@compiler-errors / @ouz-a Please let me know if you have any strong opinion here.

r? @compiler-errors

@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 Dec 6, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me

left some thoughts about code duplication, but i don't know if they're actionable... something we should think about in smir development in the long term tho.

compiler/rustc_smir/src/rustc_smir/context.rs Show resolved Hide resolved
compiler/stable_mir/src/mir/body.rs Show resolved Hide resolved
compiler/stable_mir/src/mir/body.rs Show resolved Hide resolved
compiler/stable_mir/src/mir/body.rs Outdated Show resolved Hide resolved
celinval and others added 2 commits December 6, 2023 17:48
@celinval
Copy link
Contributor Author

celinval commented Dec 7, 2023

@bors r=@compiler-errors

@bors rollup

@bors
Copy link
Contributor

bors commented Dec 7, 2023

📌 Commit 4616b9f has been approved by compiler-errors

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 Dec 7, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 7, 2023
…r-errors

Add method to get type of an Rvalue in StableMIR

Provide a method to StableMIR users to retrieve the type of an Rvalue operation. There were two possible implementation:

1. Create the logic inside stable_mir to process the type according to the Rvalue semantics, which duplicates the logic of `rustc_middle::mir::Rvalue::ty()`.
2. Implement the Rvalue translation from StableMIR back to internal representation, invoke the `rustc_middle::mir::Rvalue::ty()`, and translate the return value to StableMIR.

I chose the first one for now since the duplication was fairly small, and the option 2 would require way more work to translate everything back to rustc internal representation. If we eventually add those translations, we could easily swap to the option 2.

`@compiler-errors` / `@ouz-a` Please let me know if you have any strong opinion here.

r? `@compiler-errors`
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Dec 7, 2023
…r-errors

Add method to get type of an Rvalue in StableMIR

Provide a method to StableMIR users to retrieve the type of an Rvalue operation. There were two possible implementation:

1. Create the logic inside stable_mir to process the type according to the Rvalue semantics, which duplicates the logic of `rustc_middle::mir::Rvalue::ty()`.
2. Implement the Rvalue translation from StableMIR back to internal representation, invoke the `rustc_middle::mir::Rvalue::ty()`, and translate the return value to StableMIR.

I chose the first one for now since the duplication was fairly small, and the option 2 would require way more work to translate everything back to rustc internal representation. If we eventually add those translations, we could easily swap to the option 2.

``@compiler-errors`` / ``@ouz-a`` Please let me know if you have any strong opinion here.

r? ``@compiler-errors``
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 7, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#116420 (discard invalid spans in external blocks)
 - rust-lang#118686 (Only check principal trait ref for object safety)
 - rust-lang#118688 (Add method to get type of an Rvalue in StableMIR)
 - rust-lang#118707 (Ping GuillaumeGomez for changes in rustc_codegen_gcc)
 - rust-lang#118712 (targets: remove not-added {i386,i486}-unknown-linux-gnu)
 - rust-lang#118719 (CFI: Add char to CFI integer normalization)

Failed merges:

 - rust-lang#117586 (Uplift the (new solver) canonicalizer into `rustc_next_trait_solver`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 20ed805 into rust-lang:master Dec 8, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 8, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 8, 2023
Rollup merge of rust-lang#118688 - celinval:smir-rvalue-ty, r=compiler-errors

Add method to get type of an Rvalue in StableMIR

Provide a method to StableMIR users to retrieve the type of an Rvalue operation. There were two possible implementation:

1. Create the logic inside stable_mir to process the type according to the Rvalue semantics, which duplicates the logic of `rustc_middle::mir::Rvalue::ty()`.
2. Implement the Rvalue translation from StableMIR back to internal representation, invoke the `rustc_middle::mir::Rvalue::ty()`, and translate the return value to StableMIR.

I chose the first one for now since the duplication was fairly small, and the option 2 would require way more work to translate everything back to rustc internal representation. If we eventually add those translations, we could easily swap to the option 2.

```@compiler-errors``` / ```@ouz-a``` Please let me know if you have any strong opinion here.

r? ```@compiler-errors```
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants