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 ADT variant infomation to StableMIR and finish implementing TyKind::internal() #118516

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Dec 1, 2023

Introduce a VariantDef type and a mechanism to retrieve the definition from an AdtDef.

The VariantDef representation itself is just a combination of AdtDef and VariantIdx, which allow us to retrieve further information of a variant. I don't think we need to cache extra information for now, and we can translate on an on demand manner. I am leaving the fields public today due to rust-lang/project-stable-mir#56, but they shouldn't. For this PR, I've only added a method to retrieve the variant name, and its fields. I also added an implementation of RustcInternal that allow users to retrieve more information using Rust internal APIs.

I have also finished the implementation of RustcInternal for TyKind which fixes rust-lang/project-stable-mir#46.

Motivation

Both of these changes are needed in order to properly interpret things like projections. For example,

  • The variant definition is used to find out which variant we are downcasting to.
  • Being able to create Ty from TyKind helps for example processing each stage of a projection, like the code in place.ty().

@rustbot
Copy link
Collaborator

rustbot commented Dec 1, 2023

r? @spastorino

(rustbot has picked a reviewer for you, use r? to override)

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

rustbot commented Dec 1, 2023

This PR changes Stable MIR

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

@bors
Copy link
Contributor

bors commented Dec 2, 2023

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

This will allow us to provide methods to create `Ty` inside the stable
MIR, which can be helpful while handling pointers and other stuff.
@celinval
Copy link
Contributor Author

celinval commented Dec 4, 2023

r? @ouz-a

Do you think you can take a look at this one too? I don't know if @spastorino will have time.

@rustbot rustbot assigned ouz-a and unassigned spastorino Dec 4, 2023
@ouz-a
Copy link
Contributor

ouz-a commented Dec 4, 2023

Will try to review this week.

Copy link
Contributor

@ouz-a ouz-a left a comment

Choose a reason for hiding this comment

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

Overall looks very good, I'm happy that you provided very detailed documentation, and have a clear goal with this PR. just left a nit and a comment.

compiler/rustc_smir/src/rustc_smir/context.rs Outdated Show resolved Hide resolved
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2023
Although, we would like to avoid crashes whenever
possible, and that's why I wanted to make this API fallible. It's
looking pretty hard to do proper validation.

I think many of our APIs will unfortunately depend on the user doing
the correct thing since at the MIR level we are working on,
we expect types to have been checked already.
@ouz-a
Copy link
Contributor

ouz-a commented Dec 5, 2023

r=me when green

@celinval
Copy link
Contributor Author

celinval commented Dec 5, 2023

@bors r=@ouz-a

@bors rollup

@bors
Copy link
Contributor

bors commented Dec 5, 2023

📌 Commit 326fea0 has been approved by ouz-a

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 6, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#116496 (Provide context when `?` can't be called because of `Result<_, E>`)
 - rust-lang#117563 (docs: clarify explicitly freeing heap allocated memory)
 - rust-lang#117874 (`riscv32` platform support)
 - rust-lang#118516 (Add ADT variant infomation to StableMIR and finish implementing TyKind::internal())
 - rust-lang#118650 (add comment about keeping flags in sync between bootstrap.py and bootstrap.rs)
 - rust-lang#118664 (docs: remove rust-lang#110800 from release notes)
 - rust-lang#118669 (library: fix comment about const assert in win api)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 67d8999 into rust-lang:master Dec 6, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 6, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 6, 2023
Rollup merge of rust-lang#118516 - celinval:smir-variants, r=ouz-a

Add ADT variant infomation to StableMIR and finish implementing TyKind::internal()

Introduce a `VariantDef` type and a mechanism to retrieve the definition from an `AdtDef`.

The `VariantDef` representation itself is just a combination of `AdtDef` and `VariantIdx`, which allow us to retrieve further information of a variant. I don't think we need to cache extra information for now, and we can translate on an on demand manner. I am  leaving the fields public today due to rust-lang/project-stable-mir#56, but they shouldn't. For this PR, I've only added a method to retrieve the variant name, and its fields. I also added an implementation of `RustcInternal` that allow users to retrieve more information using Rust internal APIs.

I have also finished the implementation of `RustcInternal` for `TyKind` which fixes rust-lang/project-stable-mir#46.

## Motivation

Both of these changes are needed in order to properly interpret things like projections. For example,
- The variant definition is used to find out which variant we are downcasting to.
- Being able to create `Ty` from `TyKind` helps for example processing each stage of a projection, like the code in `place.ty()`.
adpaco-aws added a commit to model-checking/kani that referenced this pull request Dec 9, 2023
The main changes needed to make this migration besides a few method call
tweaks were:
1. Adapt `FunctionCtx` to cache information about the StableMIR version
of instance and its body.
- I also cleaned up how we were handling basic blocks which were
unnecessary.
2. Created a new ty_stable module that provide stable versions to
retrieve type information from StableMIR.
- I decided to keep these separate so it is cleaner for now. I foresee
that the type module will still rely on internal APIs for the next
little while, so separating them here made sense to me.
3. Since `Place` when retrieved from StableMIR body already comes
monomorphized, I modified the existing `codegen_place` to preemptively
monomorphize Place before converting it to a Stable version and invoking
`codegen_place_stable`.

### Call-outs

Leaving this as a draft for now since this still depends on the
following PRs to be merged into the Rust compiler:
  - rust-lang/rust#118524
  - rust-lang/rust#118516

---------

Co-authored-by: Adrian Palacios <[email protected]>
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.

Add a conversion method for going from smir::Ty to rustc::Ty
5 participants