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 more information to StableMIR Instance #118524

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Dec 2, 2023

Allow stable MIR users to retrieve an instance function signature, the index for a VTable instance and more information about its underlying definition.

These are needed to properly interpret function calls, either via VTable or direct calls. The CrateDef implementation will also allow users to emit diagnostic messages.

I also fixed a few issues that we had identified before with how we were retrieving body of things that may not have a body available.

  - Retrieve `FnSig`.
  - Implement CrateDef for InstanceDef.
  - Add VTable index for Virtual instances.
@rustbot
Copy link
Collaborator

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

rustbot commented Dec 2, 2023

This PR changes Stable MIR

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

@celinval
Copy link
Contributor Author

celinval commented Dec 2, 2023

r? @ouz-a

@ouz-a
Copy link
Contributor

ouz-a commented Dec 2, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 2, 2023

📌 Commit c997c6d 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 2, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#118514 (rustc_hir_typeck: Fix ICE when probing for non-ASCII function alternative)
 - rust-lang#118524 (Add more information to StableMIR Instance)
 - rust-lang#118528 (replace `once_cell::sync::OnceCell` with std `OnceLock`)
 - rust-lang#118539 (move packed-struct tests into packed/ folder)

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

Add more information to StableMIR Instance

Allow stable MIR users to retrieve an instance function signature, the index for a VTable instance and more information about its underlying definition.

These are needed to properly interpret function calls, either via VTable or direct calls. The `CrateDef` implementation will also allow users to emit diagnostic messages.

I also fixed a few issues that we had identified before with how we were retrieving body of things that may not have a body available.
celinval added a commit to celinval/rust-dev that referenced this pull request Dec 6, 2023
Change the implementation of `Instance::is_foreign_item` to directly
query the compiler for the instance `def_id` instead of incorrectly
relying on the conversion to `CrateItem`.

Background:

- In pull rust-lang#118524, I fixed the
  conversion from Instance to CrateItem to avoid the conversion if the
  instance didn't have a body available. This broke the `is_foreign_item`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 6, 2023
Fix is_foreign_item for StableMIR instance

Change the implementation of `Instance::is_foreign_item` to directly query the compiler for the instance `def_id` instead of incorrectly relying on the conversion to `CrateItem`. I also added a method to check if the instance has body, since the function already existed and it just wasn't exposed via public APIs. This makes it much cheaper for the user to check if the instance has body.

## Background:

- In pull rust-lang#118524, I fixed the conversion from Instance to CrateItem to avoid the conversion if the instance didn't have a body available. This broke the `is_foreign_item`.

r? `@ouz-a`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 7, 2023
Rollup merge of rust-lang#118681 - celinval:fix-foreign-item, r=ouz-a

Fix is_foreign_item for StableMIR instance

Change the implementation of `Instance::is_foreign_item` to directly query the compiler for the instance `def_id` instead of incorrectly relying on the conversion to `CrateItem`. I also added a method to check if the instance has body, since the function already existed and it just wasn't exposed via public APIs. This makes it much cheaper for the user to check if the instance has body.

## Background:

- In pull rust-lang#118524, I fixed the conversion from Instance to CrateItem to avoid the conversion if the instance didn't have a body available. This broke the `is_foreign_item`.

r? `@ouz-a`
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 <73246657+adpaco-aws@users.noreply.github.com>
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.

5 participants