Renames mir::Mir to mir::Body #60242

marimeireles · 2019-04-24T17:10:13Z

rust-highfive · 2019-04-24T17:10:16Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eddyb (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

varkor · 2019-04-24T23:28:16Z

@marimeireles: if you run x.py check, you should be able to make sure that you've replaced all the occurrences of mir::Mir (without having to actually build the entire compiler). It looks like you're missing a few at the moment.

marimeireles · 2019-04-24T23:32:06Z

Hi @varkor.
I know! Sorry for not testing before 🙈
I missed all of the objects. I'm changing them now.
Thanks for the answer.

bors · 2019-04-25T06:45:47Z

☔ The latest upstream changes (presumably #60250) made this pull request unmergeable. Please resolve the merge conflicts.

marimeireles · 2019-04-25T11:50:58Z

Hi @eddyb, can you please check my PR?
I think it's correct now.

The problem is if we don't merge it soon it will have a thousand of conflicts (like the one I just had) and other people will have conflicts too. I was thinking maybe I should warn the rust community somewhere? Do we have an email list besides the irc channels?

oli-obk · 2019-04-25T12:13:32Z

The problem is if we don't merge it soon it will have a thousand of conflicts (like the one I just had) and other people will have conflicts too. I was thinking maybe I should warn the rust community somewhere? Do we have an email list besides the irc channels?

bors will inform everyone. We can prioritize your PR once it's ready so it doesn't have to get broken over and over again.

marimeireles · 2019-04-25T12:15:20Z

@oli-obk ok.
So I'll fix it now and upload a new commit. Once it has passed all the checks successfully I'll ping you again, okay?
Thanks!

src/librustc_mir/util/borrowck_errors.rs

src/test/incremental/hashes/call_expressions.rs

src/librustc/session/config.rs

oli-obk · 2019-04-25T12:21:43Z

So I'll fix it now and upload a new commit. Once it has passed all the checks successfully I'll ping you again, okay?

If you want you can wait for a review by @eddyb and then address all the conflicts at once. Otherwise you might be rebasing a lot over the next days.

We can review even with conflicts.

marimeireles · 2019-04-25T12:24:14Z

@oli-obk Ah, okay then :)
I'll wait for @eddyb.
Thanks for the reviews.

src/librustc/mir/interpret/mod.rs

eddyb · 2019-04-26T08:17:45Z

src/librustc/mir/cache.rs

@@ -57,7 +57,7 @@ impl Cache {
    }
 }

-fn calculate_predecessors(mir: &Mir<'_>) -> IndexVec<BasicBlock, Vec<BasicBlock>> {
+fn calculate_predecessors(mir: &Body<'_>) -> IndexVec<BasicBlock, Vec<BasicBlock>> {


@oli We might need a separate issue for replacing mir used in the names of mir::Bodys (I don't think we can do too, in this PR, it would bitrot much faster then).

makes sense, we can just keep the issue open and make it track multiple points

src/librustc/mir/mod.rs

eddyb · 2019-04-26T08:19:34Z

src/librustc/session/config.rs

@@ -460,7 +460,7 @@ pub enum PrintRequest {

 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum BorrowckMode {
-    Mir,
+    Body,


This should also remain Mir (basically, if it's an enum variant, it should stay Mir, because it's referring to rustc::mir, e.g. as opposed to rustc::hir, not rustc::mir::Mir)

This doesn't seem fixed yet, looking at the diff.

eddyb · 2019-04-26T08:21:35Z

src/librustc/ty/context.rs

-    steal_mir: TypedArena<Steal<Mir<'tcx>>>,
-    mir: TypedArena<Mir<'tcx>>,
+    steal_mir: TypedArena<Steal<Body<'tcx>>>,
+    mir: TypedArena<Body<'tcx>>,


I would avoid importing Body like this, especially outside of rustc::mir, rustc_mir, or rustc_codegen_ssa, and instead refer to it as mir::Body.
Also, the names of these arena fields should probably be steal_mir_body and mir_body.

I'm gonna wait for @oli here.

yea, this should be mir::Body so it's clear from a glance what's going on.

the field renames can happen in this PR, they aren't ever touched by other PRs, so there's no churn

eddyb · 2019-04-26T08:29:44Z

src/librustc/ty/context.rs

        self.global_arenas.steal_mir.alloc(Steal::new(mir))
    }

-    pub fn alloc_mir(self, mir: Mir<'gcx>) -> &'gcx Mir<'gcx> {
+    pub fn alloc_mir(self, mir: Body<'gcx>) -> &'gcx Body<'gcx> {


These methods should probably also end in _mir_body (not sure if we should do this in this PR though).

Okay... We should wait for @oli answer, right?

you're pinging the wrong oli ;) I'm @oli-obk

yea, that rename makes sense, but let's do it in a separate PR to reduce bitrot

src/librustc_mir/transform/mod.rs

src/test/ui/nll/return_from_loop.stderr

marimeireles · 2019-04-26T14:05:13Z

Thanks for the reviews @eddyb. I'll make one commit for each kind of fix to try to keep it organized and in the end I'll smash them together.

rust-highfive · 2019-04-26T15:10:59Z

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:0e9ac420:start=1556290816020680043,finish=1556290964238484141,duration=148217804098
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[00:06:17]    Compiling rustc_macros v0.1.0 (/checkout/src/librustc_macros)
[00:06:23]    Compiling syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:06:27]    Compiling rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:07:28]    Compiling syntax_ext v0.0.0 (/checkout/src/libsyntax_ext)
[00:07:54] error[E0599]: no variant named `Body` found for type `session::config::OutputType` in the current scope
[00:07:54]    --> src/librustc/session/config.rs:162:27
[00:07:54] 142 | pub enum OutputType {
[00:07:54] 142 | pub enum OutputType {
[00:07:54]     | ------------------- variant `Body` not found here
[00:07:54] ...
[00:07:54] 162 |             | OutputType::Body
[00:07:54]     |                           ^^^^ variant not found in `session::config::OutputType`
[00:07:54] 
[00:07:54] error[E0599]: no variant named `Body` found for type `session::config::OutputType` in the current scope
[00:07:54]    --> src/librustc/session/config.rs:173:25
[00:07:54] 142 | pub enum OutputType {
[00:07:54] 142 | pub enum OutputType {
[00:07:54]     | ------------------- variant `Body` not found here
[00:07:54] ...
[00:07:54] 173 |             OutputType::Body => "mir",
[00:07:54]     |                         ^^^^ variant not found in `session::config::OutputType`
[00:07:54] 
[00:07:54] error[E0599]: no variant named `Body` found for type `session::config::OutputType` in the current scope
[00:07:54]    --> src/librustc/session/config.rs:185:34
[00:07:54] 142 | pub enum OutputType {
[00:07:54] 142 | pub enum OutputType {
[00:07:54]     | ------------------- variant `Body` not found here
[00:07:54] ...
[00:07:54] 185 |             "mir" => OutputType::Body,
[00:07:54]     |                                  ^^^^ variant not found in `session::config::OutputType`
[00:07:54] 
[00:07:54] error[E0599]: no variant named `Body` found for type `session::config::OutputType` in the current scope
[00:07:54]    --> src/librustc/session/config.rs:201:25
[00:07:54] 142 | pub enum OutputType {
[00:07:54] 142 | pub enum OutputType {
[00:07:54]     | ------------------- variant `Body` not found here
[00:07:54] ...
[00:07:54] 201 |             OutputType::Body.shorthand(),
[00:07:54]     |                         ^^^^ variant not found in `session::config::OutputType`
[00:07:54] 
[00:07:54] error[E0599]: no variant named `Body` found for type `session::config::OutputType` in the current scope
[00:07:54]    --> src/librustc/session/config.rs:214:25
[00:07:54] 142 | pub enum OutputType {
[00:07:54] 142 | pub enum OutputType {
[00:07:54]     | ------------------- variant `Body` not found here
[00:07:54] ...
[00:07:54] 214 |             OutputType::Body => "mir",
[00:07:54]     |                         ^^^^ variant not found in `session::config::OutputType`
[00:07:54] 
[00:07:54] error[E0599]: no variant named `Body` found for type `session::config::OutputType` in the current scope
[00:07:54]    --> src/librustc/session/config.rs:281:27
[00:07:54] 142 | pub enum OutputType {
[00:07:54] 142 | pub enum OutputType {
[00:07:54]     | ------------------- variant `Body` not found here
[00:07:54] ...
[00:07:54] 281 |             | OutputType::Body
[00:07:54]     |                           ^^^^ variant not found in `session::config::OutputType`
[00:08:01] error: aborting due to 6 previous errors
[00:08:01] 
[00:08:01] For more information about this error, try `rustc --explain E0599`.
[00:08:01] error: Could not compile `rustc`.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

bors · 2019-04-27T08:16:10Z

☔ The latest upstream changes (presumably #59540) made this pull request unmergeable. Please resolve the merge conflicts.

marimeireles · 2019-05-18T20:13:20Z

Sorry it took me so long to fix this. I saw that there is another person trying to fix this issue in a newer branch. I'll keep an eye on it in case their fix doesn't work.

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.

Dylan-DPC-zz · 2019-06-03T12:21:18Z

ping from triage @marimeireles you have conflicts to resolve

eddyb · 2019-06-03T18:03:11Z

@Dylan-DPC This landed as #60928, closing.

rust-highfive assigned eddyb Apr 24, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2019

This comment has been minimized.

Sign in to view

Renames mir::Mir to mir::Body

4825360

marimeireles force-pushed the master branch from 22844eb to 4825360 Compare April 25, 2019 00:04

This comment has been minimized.

Sign in to view

Lints code

daf1984

oli-obk reviewed Apr 25, 2019

View reviewed changes

src/librustc_mir/util/borrowck_errors.rs Outdated Show resolved Hide resolved

src/test/incremental/hashes/call_expressions.rs Outdated Show resolved Hide resolved

src/librustc/session/config.rs Outdated Show resolved Hide resolved

eddyb reviewed Apr 26, 2019

View reviewed changes

src/librustc/mir/interpret/mod.rs Show resolved Hide resolved

eddyb reviewed Apr 26, 2019

View reviewed changes

src/librustc/mir/mod.rs Outdated Show resolved Hide resolved

eddyb reviewed Apr 26, 2019

View reviewed changes

src/librustc_mir/transform/mod.rs Outdated Show resolved Hide resolved

eddyb reviewed Apr 26, 2019

View reviewed changes

src/test/ui/nll/return_from_loop.stderr Outdated Show resolved Hide resolved

marimeireles added 4 commits April 26, 2019 11:07

Fix merginf conflicts

36ada49

Change Body -> Mir in enums

865914b

Change comment on src/librustc_mir/transform/mod.rs

efc11fc

Change Body -> Mir on return_from_loop.stderr

ba5e4d7

marimeireles added 2 commits April 26, 2019 11:18

Change comment to function/constant body

8d8461b

Fix the comments from Body -> mir::Body

570405b

eddyb mentioned this pull request Apr 27, 2019

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

Closed

2 tasks

TheSirC mentioned this pull request May 17, 2019

Changes the type mir::Mir into mir::Body #60928

Merged

Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2019

eddyb closed this Jun 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renames mir::Mir to mir::Body #60242

Renames mir::Mir to mir::Body #60242

marimeireles commented Apr 24, 2019

rust-highfive commented Apr 24, 2019

This comment has been minimized.

varkor commented Apr 24, 2019

marimeireles commented Apr 24, 2019

This comment has been minimized.

bors commented Apr 25, 2019

marimeireles commented Apr 25, 2019

oli-obk commented Apr 25, 2019

marimeireles commented Apr 25, 2019

oli-obk commented Apr 25, 2019

marimeireles commented Apr 25, 2019

eddyb Apr 26, 2019

oli-obk Apr 27, 2019

eddyb Apr 26, 2019

marimeireles Apr 26, 2019

eddyb Apr 30, 2019

eddyb Apr 26, 2019

marimeireles Apr 26, 2019

oli-obk Apr 27, 2019

eddyb Apr 26, 2019

marimeireles Apr 26, 2019

oli-obk Apr 27, 2019

marimeireles commented Apr 26, 2019

rust-highfive commented Apr 26, 2019

bors commented Apr 27, 2019

marimeireles commented May 18, 2019

Dylan-DPC-zz commented Jun 3, 2019

eddyb commented Jun 3, 2019

Renames mir::Mir to mir::Body #60242

Renames mir::Mir to mir::Body #60242

Conversation

marimeireles commented Apr 24, 2019

rust-highfive commented Apr 24, 2019

This comment has been minimized.

varkor commented Apr 24, 2019

marimeireles commented Apr 24, 2019

This comment has been minimized.

bors commented Apr 25, 2019

marimeireles commented Apr 25, 2019

oli-obk commented Apr 25, 2019

marimeireles commented Apr 25, 2019

oli-obk commented Apr 25, 2019

marimeireles commented Apr 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marimeireles commented Apr 26, 2019

rust-highfive commented Apr 26, 2019

bors commented Apr 27, 2019

marimeireles commented May 18, 2019

Dylan-DPC-zz commented Jun 3, 2019

eddyb commented Jun 3, 2019