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

Rename mir::Mir to mir::Body, by analogy with hir::Body. #60229

Closed
2 tasks done
eddyb opened this issue Apr 24, 2019 · 11 comments
Closed
2 tasks done

Rename mir::Mir to mir::Body, by analogy with hir::Body. #60229

eddyb opened this issue Apr 24, 2019 · 11 comments
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Apr 24, 2019 •
edited by oli-obk
Loading

I think it would make more sense than the current setup, as a Mir coresponds to the body of a constant or function, in MIR.
Also, MIR encompasses more than just bodies - e.g. the miri value representation, but arguably the whole type/trait-system (including chalk) could be considered an integral part of "the Mid-level rustc IR".

cc @rust-lang/compiler
@eddyb eddyb added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Apr 24, 2019
@jonas-schievink jonas-schievink added A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 24, 2019
@marimeireles
Copy link

Hi @eddyb can I take this one this one to get my feet wet on rust?

@nikomatsakis
Copy link
Contributor

I feel pretty good about this naming. As far as process, I feel like we could probably make this specific decision rather quickly. We could also try to have a dedicated nomenclature discussion, where we look over a lot of names and try to "harmonize" them, rather than doing things piecemeal. But this particular choice feels like a clear win.

@eddyb
Copy link
Member Author

eddyb commented Apr 24, 2019

@marimeireles Sure, go ahead!

@TheSirC TheSirC mentioned this issue May 17, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue May 25, 2019
@Centril
Rollup merge of rust-lang#60928 - TheSirC:fix/60229, r=eddyb
238d621 
Changes the type `mir::Mir` into `mir::Body`

Fixes part 1 of rust-lang#60229 (previously attempted in rust-lang#60242).

I stumbled upon the issue and it seems that the previous attempt at solving it was not merged. This is a second try more up-to-date.

The commit should have changed comments as well.
At the time of writting, it passes the tidy and check tool.
Centril added a commit to Centril/rust that referenced this issue May 25, 2019
@Centril
Rollup merge of rust-lang#60928 - TheSirC:fix/60229, r=eddyb
9f210e4 
Changes the type `mir::Mir` into `mir::Body`

Fixes part 1 of rust-lang#60229 (previously attempted in rust-lang#60242).

I stumbled upon the issue and it seems that the previous attempt at solving it was not merged. This is a second try more up-to-date.

The commit should have changed comments as well.
At the time of writting, it passes the tidy and check tool.
Centril added a commit to Centril/rust that referenced this issue May 25, 2019
@Centril
Rollup merge of rust-lang#60928 - TheSirC:fix/60229, r=eddyb
4761a82 
Changes the type `mir::Mir` into `mir::Body`

Fixes part 1 of rust-lang#60229 (previously attempted in rust-lang#60242).

I stumbled upon the issue and it seems that the previous attempt at solving it was not merged. This is a second try more up-to-date.

The commit should have changed comments as well.
At the time of writting, it passes the tidy and check tool.
Centril added a commit to Centril/rust that referenced this issue May 28, 2019
@Centril
Rollup merge of rust-lang#60928 - TheSirC:fix/60229, r=eddyb
ee08261 
Changes the type `mir::Mir` into `mir::Body`

Fixes part 1 of rust-lang#60229 (previously attempted in rust-lang#60242).

I stumbled upon the issue and it seems that the previous attempt at solving it was not merged. This is a second try more up-to-date.

The commit should have changed comments as well.
At the time of writting, it passes the tidy and check tool.
@imbrem
Copy link
Contributor

imbrem commented Jun 3, 2019

Could I try part 2 of this, or is it already taken?

@varkor
Copy link
Member

varkor commented Jun 3, 2019

@imbrem: it looks like no-one else is working on this, so go ahead!

@eddyb
Copy link
Member Author

eddyb commented Jun 3, 2019

Not sure it should be mir_body - in MIR-specific code, such as all of rustc::mir and rustc_mir, it could just be body, as it would be unambiguous.

@imbrem
Copy link
Contributor

imbrem commented Jun 3, 2019

@eddyb I agree. I'll get to it in a few hours or so!

@imbrem
Copy link
Contributor

imbrem commented Jun 3, 2019

Oh also, what do I do about the lifetime 'mir. Just leave it as is?

@imbrem imbrem mentioned this issue Jun 3, 2019
Merged
bors added a commit that referenced this issue Jun 10, 2019
@bors
Auto merge of #61506 - imbrem:mir_body_renaming, r=eddyb
1cbd8a4 
Changed usages of `mir` in librustc::mir and librustc_mir to `body`

Work on part 2 of #60229
@JohnTitor
Copy link
Member

Does #61506 make the second checkbox checked? @eddyb

@imbrem
Copy link
Contributor

imbrem commented Jul 25, 2019

@JohnTitor that was certainly my intention haha

@oli-obk oli-obk closed this as completed Jul 27, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Jul 27, 2019

Yea this looks resolved to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@eddyb @nikomatsakis @oli-obk @jonas-schievink @varkor @marimeireles @JohnTitor @imbrem