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

HirId-ification, continued #51321

Merged
merged 2 commits into from
Jul 2, 2018

Conversation

zackmdavis
Copy link
Member

Another incremental step towards the vision of #50928 (previously: #50929).

r? @michaelwoerister

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2018
@rust-highfive

This comment has been minimized.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 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.
[00:47:17] ...........................................................................i........................
[00:47:22] ....................................................................................................
[00:47:28] ....................................................................................................
[00:47:35] ....................................................................................................
[00:47:40] ........i.................iiiiiiiii...................................................
[00:47:40] 
[00:47:40] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:48:35] ...........................................................................i........................
[00:48:40] ....................................................................................................
[00:48:45] ....................................................................................................
[00:48:51] ....................................................................................................
[00:48:56] ........i.................iiiiiiiii...................................................
[00:48:56] 
[00:48:56]  finished in 76.529
[00:48:56] travis_fold:end:test_ui_nll

---
travis_time:start:test_mir-opt
Check compiletest suite=mir-opt mode=mir-opt (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:01:35] 
[01:01:35] running 50 tests
[01:01:40] ERROR 2018-06-03T06:03:47Z: compiletest::runtest: None
[01:01:40] ERROR 2018-06-03T06:03:47Z: compiletest::runtest: None
[01:01:41] ERROR 2018-06-03T06:03:48Z: compiletest::runtest: None
[01:01:42] ERROR 2018-06-03T06:03:49Z: compiletest::runtest: None
[01:01:42] ERROR 2018-06-03T06:03:49Z: compiletest::runtest: None
[01:01:53] ERROR 2018-06-03T06:04:01Z: compiletest::runtest: Some("    bb0: {")
  }
[01:01:57]     bb1: {                              
[01:01:57]         falseUnwind -> [real: bb2, cleanup: bb3];
[01:01:57]     }
[01:01:57]     bb2: {                              
[01:01:57]         StorageLive(_2);
[01:01:57]         StorageLive(_3);
[01:01:57]         StorageLive(_4);
[01:01:57]         _4 = std::option::Option<&'36_0rs S<'36_0rs>>::None;
[01:01:57]         _3 = const <std::cell::Cell<T>>::new(move _4) -> [return: bb4, unwind: bb3];
[01:01:57]     bb3: {
[01:01:57]         resume;
[01:01:57]     }
[01:01:57]     }
[01:01:57]     bb4: {                              
[01:01:57]         StorageDead(_4);
[01:01:57]         _2 = S<'36_0rs> { r: move _3 };
[01:01:57]         StorageDead(_3);
[01:01:57]         StorageLive(_6);
[01:01:57]         _6 = &'18s (_2.0: std::cell::Cell<std::option::Option<&'36_0rs S<'36_0rs>>>);
[01:01:57]         StorageLive(_7);
[01:01:57]         StorageLive(_8);
[01:01:57]         StorageLive(_9);
[01:01:57]         _9 = &'36_0rs _2;
[01:01:57]         _8 = &'36_0rs (*_9);
[01:01:57]         _7 = std::option::Option<&'36_0rs S<'36_0rs>>::Some(move _8,);
[01:01:57]         StorageDead(_8);
[01:01:57]         _5 = const <std::cell::Cell<T>>::set(move _6, move _7) -> [return: bb5, unwind: bb3];
[01:01:57]     }
[01:01:57]     bb5: {                              
[01:01:57]         EndRegion('18s);
[01:01:57]         StorageDead(_7);
[01:01:57]         StorageDead(_6);
[01:01:57]         StorageDead(_9);
[01:01:57]         StorageLive(_11);
[01:01:57]         _11 = const query() -> [return: bb6, unwind: bb3];
[01:01:57]     }
[01:01:57]     bb6ased mut _1;
[01:01:57]         Validate(Acquire, [(*_6): i32/ReScope(Node(ItemLocalId(10)))]);
[01:01:57]         Validate(Suspend(ReScope(Node(ItemLocalId(10)))), [(*_6): i32/ReScope(Node(ItemLocalId(10)))]);
[01:01:57]         _5 = &ReErased mut (*_6);
[01:01:57]         Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(10)))]);
[01:01:57]         Validate(Release, [_2: (), _3: &ReScope(Node(ItemLocalId(10))) Test, _5: &ReScope(Node(ItemLocalId(10))) mut i32]);
[01:01:57]         _2 = const Test::foo(move _3, move _5) -> bb1;
[01:01:57]     bb1: {
[01:01:57]     bb1: {
[01:01:57]         Validate(Acquire, [_2: ()]);
[01:01:57]         EndRegion(ReScope(Node(ItemLocalId(10))));
[01:01:57] ... (elided)
[01:01:57]         return;
[01:01:57] }
[01:01:57] Actual:
[01:01:57] Actual:
[01:01:57] fn main() -> (){
[01:01:57]     let mut _0: ();
[01:01:57]     scope 1 {
[01:01:57]         scope 3 {
[01:01:57]         scope 4 {
[01:01:57]         scope 4 {
[01:01:57]             let _7: [closure@NodeId(50)];
[01:01:57]     }
[01:01:57]     scope 2 {
[01:01:57]     scope 2 {
[01:01:57]         let mut _1: i32;
[01:01:57]     }
[01:01:57]     let mut _2: ();
[01:01:57]     let mut _3: &ReErased Test;
[01:01:57]     let mut _4: Test;
[01:01:57]     let mut _5: &ReErased mut i32;
[01:01:57]     let mut _6: &ReErased mut i32;
[01:01:57]     let mut _8: i32;
[01:01:57]     let mut _9: &ReErased [closure@NodeId(50)];
[01:01:57]     let mut _10: (&ReErased mut i32,);
[01:01:57]     let mut _11: &ReErased mut i32;
[01:01:57]     let mut _12: &ReErased mut i32;
[01:01:57]     let mut _13: &ReErased Test;
[01:01:57]     (35)))), [_7: [closure@NodeId(50)]]);
[01:01:57]         _9 = &ReErased _7;
[01:01:57]         Validate(Acquire, [(*_9): [closure@NodeId(50)]/ReScope(Node(ItemLocalId(35))) (imm)]);
[01:01:57]         StorageLive(_10);
[01:01:57]         StorageLive(_11);
[01:01:57]         StorageLive(_12);
[01:01:57]         Validate(Suspend(ReScope(Node(ItemLocalId(35)))), [_1: i32]);
[01:01:57]         _12 = &ReErased mut _1;
[01:01:57]         Validate(Acquire, [(*_12): i32/ReScope(Node(ItemLocalId(35)))]);
[01:01:57]         Validate(Suspend(ReScope(Node(ItemLocalId(35)))), [(*_12): i32/ReScope(Node(ItemLocalId(35)))]);
[01:01:57]         _11 = &ReErased mut (*_12);
[01:01:57]         Validate(Acquire, [(*_11): i32/ReScope(Node(ItemLocalId(35)))]);
[01:01:57]         _10 = (move _11,);
[01:01:57]         Validate(Release, [_8: i32, _9: &ReScope(Node(ItemLocalId(35))) [closure@NodeId(50)], _10: (&ReScope(Node(ItemLocalId(35))) mut i32,)]);
[01:01:57]         _8 = const std::ops::Fn::call(move _9, move _10) -> bb2;
[01:01:57]     }
[01:01:57]     bb2: {                              
[01:01:57]         Validate(Acquire, [_8: i32]);
[01:01:57]         EndRegion(ReScope(Node(ItemLocalId(35))));
[01:01:57]         StorageDead(_10);
[01:01:57]         StorageDead(_11);
[01:01:57]         StorageDead(_9);
[01:01:57]         StorageDead(_12);
[01:01:57]         _0 = ();
[01:01:57]         StorageDead(_7);
[01:01:57]         StorageDead(_1);
[01:01:57]         return;
[01:01:57] }', tools/compiletest/src/runtest.rs:2812:13
[01:01:57] 
[01:01:57] ---- [mir-opt] mir-opt/validate_3.rs stdout ----
[01:0Sun, 03 Jun 2018 06:04:05 GMT

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)

@michaelwoerister
Copy link
Member

r? @eddyb, maybe you could help to fix the mir-opt tests. They probably should use the printed def-path instead of the Debug output of DefId. That would make them a lot more stable.

@eddyb
Copy link
Member

eddyb commented Jun 5, 2018

@michaelwoerister What do you mean? All I'm seeing is changes in scopes, not DefIds.

@michaelwoerister
Copy link
Member

Sorry, @eddyb, yeah, might be a bug then? I didn't look too hard since I'm on parental leave :)

@eddyb
Copy link
Member

eddyb commented Jun 5, 2018

Maybe one of the calls to next_id() in HIR lowering is now scoped differently and/or one of the variables its result is bound to shadows other variables with the same name.

@pietroalbini
Copy link
Member

(note for future triage: the author has not so much time for rust at the moment, let's keep this open though #51149 (comment))

@bors
Copy link
Contributor

bors commented Jun 18, 2018

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

@zackmdavis
Copy link
Member Author

(most recent rebase just fixes conflicts, tests not expected to pass yet)

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 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.

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
Copy link
Contributor

bors commented Jun 28, 2018

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

(The present author fears not being knowledgeable enough to rewrite the
comments unilaterally; merely calling it out is a lazy half-measure, but
at least doesn't actively make things worse the way an ill-informed
rewrite would.)
@zackmdavis
Copy link
Member Author

@michaelwoerister Congratulations on the baby!!

@eddyb So, the original version of this PR (which tried to add hir_id fields to all remaining HIR nodes that didn't have them) was trying to do too many things at once, such that I wasn't able to debug it. (I tried redoing the lowering changes from scratch while paying attention to scoping, and I still ended up hitting a broken-MIR assertion.) But it turns out that the later part of the PR making mem_categorization::cmt use a HirId is independently viable, so I've rebased just that, which I think is ready for your review.

@eddyb
Copy link
Member

eddyb commented Jul 2, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Jul 2, 2018

📌 Commit f23d90a has been approved by eddyb

@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 Jul 2, 2018
@michaelwoerister
Copy link
Member

Thanks, @zackmdavis! I'm back from leave now.

@bors
Copy link
Contributor

bors commented Jul 2, 2018

⌛ Testing commit f23d90a with merge b58b721...

bors added a commit that referenced this pull request Jul 2, 2018
HirId-ification, continued

Another incremental step towards the vision of #50928 (previously: #50929).

r? @michaelwoerister
@bors
Copy link
Contributor

bors commented Jul 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing b58b721 to master...

@bors bors merged commit f23d90a into rust-lang:master Jul 2, 2018
@zackmdavis zackmdavis deleted the hiridification_generations branch July 8, 2018 01:59
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants