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.

Sign up for GitHub

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

Jump to bottom

Add new interface to smir #115187

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Add new interface to smir #115187

merged 1 commit into from
Aug 29, 2023

Conversation

ouz-a
Copy link
Contributor

@ouz-a ouz-a commented Aug 24, 2023

Removes the boiler plate from crate-info.rs, and creates new interface for the smir.

Addressing rust-lang/project-stable-mir#23

r? @spastorino

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

rustbot commented Aug 24, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

spastorino
spastorino approved these changes Aug 25, 2023
View reviewed changes
Copy link
Member

@spastorino spastorino left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good to me. I'd squash the last commit with the first.

Assigning to Celina/Oli so they can confirm this is also the idea they had in mimd.

r? @oli-obk @celinval

@rustbot
Copy link
Collaborator

rustbot commented Aug 25, 2023

Failed to set assignee to celinval: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot
Copy link
Collaborator

rustbot commented Aug 25, 2023

Could not assign reviewer from: oli-obk.
User(s) oli-obk are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

celinval
celinval reviewed Aug 26, 2023
View reviewed changes
tests/ui-fulldeps/stable-mir/crate-info.rs
// No need to keep going.
Compilation::Stop
}
rustc_internal::StableMir::new(args, test_stable_mir).run();
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the piecemeal comments. I was playing with this code while creating a test driver for stable-mir. I was wondering if we could move this to the stable_mir module.

Copy link
Contributor

Choose a reason for hiding this comment

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

see #115187 (comment) I think it should stay in rustc_internal for now.

@ouz-a ouz-a force-pushed the smir_wrap branch 2 times, most recently from c2c1e7e to c71443d Compare August 26, 2023 11:04
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
oli-obk
oli-obk approved these changes Aug 28, 2023
View reviewed changes
@spastorino
Copy link
Member

spastorino commented Aug 28, 2023 •
edited
Loading

Shouldn't we move args as a parameter of run? ... well unless you take ownership of self as Celina suggested.

@ouz-a ouz-a force-pushed the smir_wrap branch 2 times, most recently from 9c3b71f to 031a961 Compare August 29, 2023 09:27
oli-obk
oli-obk reviewed Aug 29, 2023
View reviewed changes
compiler/rustc_smir/src/stable_mir/mod.rs Outdated
Comment on lines 19 to 24
use crate::rustc_internal;
use crate::rustc_smir::Tables;
use rustc_driver::{Callbacks, Compilation, RunCompiler};
use rustc_interface::{interface, Queries};
use rustc_middle::ty::TyCtxt;
use rustc_session::EarlyErrorHandler;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not have any rustc_* stuff inside the stable_mir modules. Everything should be abstracted via the Context trait. Otherwise we will not be able to build the SMIR crate on stable on crates.io in the future.

In this case, we cannot abstract it, because in order to get a Context instance, we first need to start up rustc. Let's figure out how to handle this best separately. for now, move this logic to where rustc_internal::run lives.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Aug 29, 2023

@bors delegate+

r= me with CI fixed and commits squashed

@bors
Copy link
Collaborator

bors commented Aug 29, 2023

✌️ @ouz-a, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@ouz-a
Copy link
Contributor Author

ouz-a commented Aug 29, 2023

@bors r=@oli-obk

@bors
Copy link
Collaborator

bors commented Aug 29, 2023

📌 Commit c2fe0bf 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 Aug 29, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 29, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2023
@bors
Auto merge of rust-lang#115354 - matthiaskrgr:rollup-4cotcxz, r=matth…
83995f3 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#111580 (Don't ICE on layout computation failure)
 - rust-lang#114923 (doc: update lld-flavor ref)
 - rust-lang#115174 (tests: add test for rust-lang#67992)
 - rust-lang#115187 (Add new interface to smir)
 - rust-lang#115300 (Tweaks and improvements on SMIR around generics_of and predicates_of)
 - rust-lang#115340 (some more is_zst that should be is_1zst)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 61c367c into rust-lang:master Aug 29, 2023
@rustbot rustbot added this to the 1.74.0 milestone Aug 29, 2023
@ouz-a ouz-a deleted the smir_wrap branch August 30, 2023 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk approved these changes

@bjorn3 bjorn3 bjorn3 left review comments

@celinval celinval celinval left review comments

@spastorino spastorino spastorino approved these changes

Assignees

@spastorino spastorino

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
Milestone
1.74.0
Development

Successfully merging this pull request may close these issues.
8 participants
@ouz-a @rustbot @rust-log-analyzer @spastorino @oli-obk @bors @bjorn3 @celinval