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

Move stuff around on stable_mir and rustc_smir crate #118266

Merged
merged 3 commits into from
Nov 25, 2023

Conversation

celinval
Copy link
Contributor

  1. Break down rustc_smir/mod.rs file.
    • This file was getting too big and causing a lot of merge conflicts.
      All these changes shouldn't be visible to users since this module is private.
  2. Move the compiler interface defs to its own module
    • Separate items that are exposed in the stable_mir crate to be used
      by the compiler from items that we expect to be used by tool developers.

This file was getting too big and causing a lot of merge conflicts. All
these changes shouldn't be visible to users since this module is private.
Separate items that are exposed in the `stable_mir` crate to be used
by the compiler from items that we expect to be used by tool developers.
@rustbot
Copy link
Collaborator

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

rustbot commented Nov 24, 2023

This PR changes Stable MIR

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

Comment on lines 20 to 30
use self::ty::{ImplDef, ImplTrait, IndexedVal, Span, TraitDecl, TraitDef, Ty};
pub(crate) use crate::compiler_interface::with;
pub use crate::crate_def::CrateDef;
pub use crate::crate_def::DefId;
use crate::mir::pretty::function_name;
use crate::mir::Body;
use crate::mir::Mutability;
pub use error::*;
use std::fmt;
use std::fmt::Debug;
use std::{cell::Cell, io};

use self::ty::{
GenericPredicates, Generics, ImplDef, ImplTrait, IndexedVal, LineInfo, Span, TraitDecl,
TraitDef, Ty, TyKind,
};
use std::io;
Copy link
Member

Choose a reason for hiding this comment

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

Microscopic nit: Since you're already touching all the imports, could you organize them by StdExternCrate order?

use std::fmt;
use std::fmt::Debug;
use std::io;

pub(crate) use compiler_interface::with;
pub use crate_def::{CrateDef, DefId};
pub use error::*;
use mir::pretty::function_name;
use mir::{Body, Mutability};
use ty::{ImplDef, ImplTrait, IndexedVal, Span, TraitDecl, TraitDef, Ty};

Copy link
Member

Choose a reason for hiding this comment

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

Also, why is with re-exported here? You could probably just use crate::compiler_interface::with in all the dependency files, since compiler_interface is pub itself?

Copy link
Contributor Author

@celinval celinval Nov 24, 2023

Choose a reason for hiding this comment

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

Microscopic nit: Since you're already touching all the imports, could you organize them by StdExternCrate order?

Yes, I can re-organize the imports. I basically just used my IDE (clion) to organize the imports, and I believe it follows similar convention to https://doc.rust-lang.org/beta/style-guide/items.html. Question, is the StdExternCrate a compiler style preference or a Rust one?

Also, why is with re-exported here? You could probably just use crate::compiler_interface::with in all the dependency files, since compiler_interface is pub itself?

Will do.

Copy link
Member

Choose a reason for hiding this comment

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

Question, is the StdExternCrate a compiler style preference or a Rust one?

It's widely used in Rust, but to be honest, it's mostly a compiler-errors preference 😆

@compiler-errors
Copy link
Member

anyways r=me with or without making the imports easier to read, unless you want a specific review from santiago or oguz

@celinval
Copy link
Contributor Author

@bors r=@compiler-errors

@bors
Copy link
Contributor

bors commented Nov 24, 2023

📌 Commit f8c2478 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 Nov 24, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 25, 2023
Rollup of 9 pull requests

Successful merges:

 - rust-lang#118220 (general improvements/fixes on bootstrap)
 - rust-lang#118251 (rustdoc-search: avoid infinite where clause unbox)
 - rust-lang#118253 (Replace `option.map(cond) == Some(true)` with `option.is_some_and(cond)`)
 - rust-lang#118255 (Request that rust-analyzer changes are sent upstream first if possible)
 - rust-lang#118259 (Move EagerResolution to rustc_infer::infer::resolve)
 - rust-lang#118262 (Relate Inherent Associated Types using eq)
 - rust-lang#118266 (Move stuff around on `stable_mir` and `rustc_smir` crate)
 - rust-lang#118271 (Separate `NaN`/`Inf` floats with `_`)
 - rust-lang#118274 (Fix smir's `Ty::Ref` pretty printing)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f513bd into rust-lang:master Nov 25, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Nov 25, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 25, 2023
Rollup merge of rust-lang#118266 - celinval:smir-break-files, r=compiler-errors

Move stuff around on `stable_mir` and `rustc_smir` crate

1. Break down rustc_smir/mod.rs file.
    - This file was getting too big and causing a lot of merge conflicts.
      All these changes shouldn't be visible to users since this module is private.
2.  Move the compiler interface defs to its own module
    - Separate items that are exposed in the `stable_mir` crate to be used
      by the compiler from items that we expect to be used by tool developers.
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