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

StableMIR: Proof-of-concept implementation + test #108846

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Mar 7, 2023

This PR is part of the project Stable MIR. The PR deletes old re-exports from rustc_smir and introduces a proof-of-concept implementation for APIs to retrieve crate information.

The implementation follows the design described here, but instead of using separate crates for the implementation, it uses separate modules inside rustc_smir.

The API introduced at this point should be seen just as an example on how we are planning to structure the communication between tools and the compiler.

I have not explored yet what should be the right granularity, the best starting point for users, neither the best way to implement it.

r? @oli-obk

This approach didn't seem to work well.
+ Add some information to the README.md
@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 Mar 7, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2023

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.


use crate::stable_mir::CrateItem;

pub type DefId = rustc_span::def_id::DefId;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be wrapped in a newtype to avoid leaking implementation details?

Copy link
Contributor

Choose a reason for hiding this comment

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

This module is specifically be forever unstable so that we allow nightly users to do more than what stable-mir allows, while still mostly using the stable-mir API

Comment on lines 34 to 49
let mod_id = DefId { index: CRATE_DEF_INDEX, krate: crate_num };
let items = if is_local {
tcx.hir_module_items(mod_id.expect_local())
.items()
.map(|item| {
let def_id = item.owner_id.def_id.to_def_id();
stable_mir::CrateItem(def_id)
})
.collect()
} else {
tcx.module_children(mod_id)
.iter()
.filter_map(|item| item.res.opt_def_id())
.map(stable_mir::CrateItem)
.collect::<Vec<_>>()
};
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fairly expensive. Do we really need this all the time?

I would expect you need a list of all (public?) mir bodies for the local crate (tcx.mir_keys(())). When looking at them, you get more DefIds (that may refer to private items or items from other crates), and you can use those DefIds to directly look up the mir body.

Do you know of any use cases for knowing all the items from another crate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Kani, we do need to see all functions in the local crate. But we definitely don't need to do this all the time. This is really just a proof of concept, and I totally agree that expensive logic shouldn't run by default.

Comment on lines +44 to +52
/// Access to the local crate.
pub fn local_crate() -> Crate {
crate::rustc_smir::local_crate()
}

/// Try to find a crate with the given name.
pub fn find_crate(name: &str) -> Option<Crate> {
crate::rustc_smir::find_crate(name)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll have to think some more of how to do this crate split in the future. The rustc_smir crate will see a different version of stable_mir than users who will compile stable_mir from crates.io.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally. I did not incorporate anything related to version yet.


use crate::stable_mir::CrateItem;

pub type DefId = rustc_span::def_id::DefId;
Copy link
Contributor

Choose a reason for hiding this comment

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

This module is specifically be forever unstable so that we allow nightly users to do more than what stable-mir allows, while still mostly using the stable-mir API

@oli-obk
Copy link
Contributor

oli-obk commented Mar 7, 2023

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 7, 2023

📌 Commit 5eaeb71 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 Mar 7, 2023
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 8, 2023
StableMIR: Proof-of-concept implementation + test

This PR is part of the [project Stable MIR](https://github.com/rust-lang/project-stable-mir). The PR deletes old re-exports from rustc_smir and introduces a proof-of-concept implementation for APIs to retrieve crate information.

The implementation follows the [design described here](https://hackmd.io/XhnYHKKuR6-LChhobvlT-g?view), but instead of using separate crates for the implementation, it uses separate modules inside `rustc_smir`.

The API introduced at this point should be seen just as an example on how we are planning to structure the communication between tools and the compiler.

I have not explored yet what should be the right granularity, the best starting point for users, neither the best way to implement it.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 8, 2023
StableMIR: Proof-of-concept implementation + test

This PR is part of the [project Stable MIR](https://github.com/rust-lang/project-stable-mir). The PR deletes old re-exports from rustc_smir and introduces a proof-of-concept implementation for APIs to retrieve crate information.

The implementation follows the [design described here](https://hackmd.io/XhnYHKKuR6-LChhobvlT-g?view), but instead of using separate crates for the implementation, it uses separate modules inside `rustc_smir`.

The API introduced at this point should be seen just as an example on how we are planning to structure the communication between tools and the compiler.

I have not explored yet what should be the right granularity, the best starting point for users, neither the best way to implement it.

r? ``@oli-obk``
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 8, 2023
StableMIR: Proof-of-concept implementation + test

This PR is part of the [project Stable MIR](https://github.com/rust-lang/project-stable-mir). The PR deletes old re-exports from rustc_smir and introduces a proof-of-concept implementation for APIs to retrieve crate information.

The implementation follows the [design described here](https://hackmd.io/XhnYHKKuR6-LChhobvlT-g?view), but instead of using separate crates for the implementation, it uses separate modules inside `rustc_smir`.

The API introduced at this point should be seen just as an example on how we are planning to structure the communication between tools and the compiler.

I have not explored yet what should be the right granularity, the best starting point for users, neither the best way to implement it.

r? ```@oli-obk```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 8, 2023
StableMIR: Proof-of-concept implementation + test

This PR is part of the [project Stable MIR](https://github.com/rust-lang/project-stable-mir). The PR deletes old re-exports from rustc_smir and introduces a proof-of-concept implementation for APIs to retrieve crate information.

The implementation follows the [design described here](https://hackmd.io/XhnYHKKuR6-LChhobvlT-g?view), but instead of using separate crates for the implementation, it uses separate modules inside `rustc_smir`.

The API introduced at this point should be seen just as an example on how we are planning to structure the communication between tools and the compiler.

I have not explored yet what should be the right granularity, the best starting point for users, neither the best way to implement it.

r? ````@oli-obk````
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Mar 8, 2023
StableMIR: Proof-of-concept implementation + test

This PR is part of the [project Stable MIR](https://github.com/rust-lang/project-stable-mir). The PR deletes old re-exports from rustc_smir and introduces a proof-of-concept implementation for APIs to retrieve crate information.

The implementation follows the [design described here](https://hackmd.io/XhnYHKKuR6-LChhobvlT-g?view), but instead of using separate crates for the implementation, it uses separate modules inside `rustc_smir`.

The API introduced at this point should be seen just as an example on how we are planning to structure the communication between tools and the compiler.

I have not explored yet what should be the right granularity, the best starting point for users, neither the best way to implement it.

r? `````@oli-obk`````
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#108686 (rustdoc: include link on all.html location header)
 - rust-lang#108846 (StableMIR: Proof-of-concept implementation + test )
 - rust-lang#108873 (Simplify `sort_by` calls)
 - rust-lang#108883 (Suppress copy impl error when post-normalized type references errors)
 - rust-lang#108884 (Tweak illegal `Copy` impl message)
 - rust-lang#108887 (Rename `MapInPlace` as `FlatMapInPlace`.)
 - rust-lang#108901 (fix: evaluate with wrong obligation stack)
 - rust-lang#108903 (Move Clippy tests back to their intended directory)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9b6b7a3 into rust-lang:master Mar 9, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 9, 2023
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