Don't emit cannot move errors twice in migrate mode #55221

matthewjasper · 2018-10-20T09:11:06Z

bors · 2018-10-21T14:52:04Z

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

pnkfelix

@bors r+

pnkfelix · 2018-10-22T07:50:56Z

src/librustc_mir/borrow_check/mod.rs

-                if let Err(_place_err) = self.is_mutable(place, is_local_mutation_allowed) {
+                if let (Err(_place_err), true) = (
+                    self.is_mutable(place, is_local_mutation_allowed),
+                    self.errors_buffer.is_empty()


Ah great idea to let the ICE through if we have already signaled errors. Or at least I hope it’s a great idea.

(ugh I clearly misread the code 3 hours ago.)

pnkfelix · 2018-10-22T07:57:33Z

@bors r+

bors · 2018-10-22T09:25:40Z

📌 Commit b375728 has been approved by pnkfelix

pnkfelix · 2018-10-22T10:20:51Z

src/librustc_mir/borrow_check/mod.rs

-                if let Err(_place_err) = self.is_mutable(place, is_local_mutation_allowed) {
+                if let (Err(_place_err), true) = (
+                    self.is_mutable(place, is_local_mutation_allowed),
+                    self.errors_buffer.is_empty()


just to be clear: this filter is going to affect more than just the migrate mode, right?

that is, I assume this will also cause us to stop emitting some move errors even in normal NLL mode?

Or rather ... I guess it will stop us ... from ICE'ing in some scenarios under normal NLL mode if we've emitted an error already ...?

I don't know how I feel about that. I guess its fine.

pnkfelix · 2018-10-22T10:21:52Z

Ugh sorry I had used my phone and I guess the review comment system there doesn't bubble out to here in a manner that bors can see!

pnkfelix · 2018-10-22T10:23:27Z

Ugh sorry I had used my phone and I guess the review comment system there doesn't bubble out to here in a manner that bors can see!

bors · 2018-10-22T18:26:24Z

⌛ Testing commit b375728 with merge fca50e8...

@pnkfelix

…, r=pnkfelix Don't emit cannot move errors twice in migrate mode Closes #55154 cc #53004 r? @pnkfelix

matthewjasper · 2018-10-22T19:58:43Z

CI doesn't appear to be actually running. does
@bors retry
reset it? no

Aaron1011 · 2018-10-24T00:07:32Z

It looks like Bors might be stuck - all commits on the auto branch have their CI checks passing.

pnkfelix · 2018-10-24T06:13:13Z

@bors r+

bors · 2018-10-24T06:13:14Z

💡 This pull request was already approved, no need to approve it again.

This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

bors · 2018-10-24T06:13:15Z

📌 Commit b375728 has been approved by pnkfelix

@pnkfelix

…te-messages, r=pnkfelix Don't emit cannot move errors twice in migrate mode Closes rust-lang#55154 cc rust-lang#53004 r? @pnkfelix

bors · 2018-10-29T01:45:48Z

⌛ Testing commit b375728 with merge 7d04c95f0da7486f2a652fb7efd4e52fe01786ef...

bors · 2018-10-29T02:37:20Z

💔 Test failed - status-appveyor

kennytm · 2018-10-29T09:25:54Z

Needs to bless a file.

---- [ui (nll)] ui\unsized-locals\unsized-exprs2.rs stdout ----
diff of stderr:
4	LL |     udrop::<[u8]>(foo()[..]);
5	   |                   ^^^^^^^^^ cannot move out of here
6	
-	error[E0507]: cannot move out of data in a `&` reference
-	  --> $DIR/unsized-exprs2.rs:22:19
-	   |
-	LL |     udrop::<[u8]>(foo()[..]);
-	   |                   ^^^^^^^^^
-	   |                   |
-	   |                   cannot move out of data in a `&` reference
-	   |                   cannot move
+	error: aborting due to previous error
15	
-	error: aborting due to 2 previous errors
-	
-	Some errors occurred: E0507, E0508.
-	For more information about an error, try `rustc --explain E0507`.
+	For more information about this error, try `rustc --explain E0508`.
20

matthewjasper · 2018-10-29T20:24:00Z

@bors r=pnkfelix

bors · 2018-10-29T20:24:01Z

📌 Commit 42a541e has been approved by pnkfelix

bors · 2018-10-29T21:25:18Z

⌛ Testing commit 42a541e with merge 6e0a5b63d7e24d392bb2b9139bd9a4e61493da48...

bors · 2018-10-30T00:28:55Z

💔 Test failed - status-appveyor

kennytm · 2018-10-30T03:01:53Z

@bors retry

3 hour timeout

bors · 2018-10-30T03:58:00Z

⌛ Testing commit 42a541e with merge b1ca390...

@pnkfelix

…, r=pnkfelix Don't emit cannot move errors twice in migrate mode Closes #55154 cc #53004 r? @pnkfelix

bors · 2018-10-30T06:34:44Z

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

rust-highfive assigned pnkfelix Oct 20, 2018

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

matthewjasper force-pushed the fewer-duplicate-migrate-messages branch from 38b0425 to b375728 Compare October 21, 2018 17:14

pnkfelix approved these changes Oct 22, 2018

View reviewed changes

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 Oct 22, 2018

pnkfelix reviewed Oct 22, 2018

View reviewed changes

bors added a commit that referenced this pull request Oct 22, 2018

Auto merge of #55221 - matthewjasper:fewer-duplicate-migrate-messages…

fca50e8

…, r=pnkfelix Don't emit cannot move errors twice in migrate mode Closes #55154 cc #53004 r? @pnkfelix

pnkfelix closed this Oct 23, 2018

pnkfelix reopened this Oct 23, 2018

This was referenced Oct 24, 2018

NLL: need 3rd round of review comparing .stderr and .nll.stderr output #54528

Closed

NLL migrate mode: issues two errors for move out of single static item #55309

Closed

NLL migrate mode: issues two errors for single move out of borrow #55310

Closed

pietroalbini mentioned this pull request Oct 25, 2018

Rollup of 15 pull requests #55357

Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 29, 2018

kennytm 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 Oct 29, 2018

Don't emit cannot move errors twice in migrate mode

42a541e

matthewjasper force-pushed the fewer-duplicate-migrate-messages branch from b375728 to 42a541e Compare October 29, 2018 20:22

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 29, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 30, 2018

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 Oct 30, 2018

bors added a commit that referenced this pull request Oct 30, 2018

Auto merge of #55221 - matthewjasper:fewer-duplicate-migrate-messages…

b1ca390

…, r=pnkfelix Don't emit cannot move errors twice in migrate mode Closes #55154 cc #53004 r? @pnkfelix

bors merged commit 42a541e into rust-lang:master Oct 30, 2018

pnkfelix mentioned this pull request Nov 6, 2018

NLL migration mode sometimes prints more diagnostics than either AST-borrowck or NLL #53004

Closed

matthewjasper deleted the fewer-duplicate-migrate-messages branch November 30, 2018 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't emit cannot move errors twice in migrate mode #55221

Don't emit cannot move errors twice in migrate mode #55221

matthewjasper commented Oct 20, 2018

bors commented Oct 21, 2018

pnkfelix left a comment

pnkfelix Oct 22, 2018

pnkfelix Oct 22, 2018

pnkfelix commented Oct 22, 2018

bors commented Oct 22, 2018

pnkfelix Oct 22, 2018

pnkfelix Oct 22, 2018

pnkfelix commented Oct 22, 2018

pnkfelix commented Oct 22, 2018

bors commented Oct 22, 2018

matthewjasper commented Oct 22, 2018 •

edited

Loading

Aaron1011 commented Oct 24, 2018

pnkfelix commented Oct 24, 2018

bors commented Oct 24, 2018

bors commented Oct 24, 2018

bors commented Oct 29, 2018

bors commented Oct 29, 2018

kennytm commented Oct 29, 2018

matthewjasper commented Oct 29, 2018

bors commented Oct 29, 2018

bors commented Oct 29, 2018

bors commented Oct 30, 2018

kennytm commented Oct 30, 2018

bors commented Oct 30, 2018

bors commented Oct 30, 2018

Don't emit cannot move errors twice in migrate mode #55221

Don't emit cannot move errors twice in migrate mode #55221

Conversation

matthewjasper commented Oct 20, 2018

bors commented Oct 21, 2018

pnkfelix left a comment

Choose a reason for hiding this comment

pnkfelix Oct 22, 2018

Choose a reason for hiding this comment

pnkfelix Oct 22, 2018

Choose a reason for hiding this comment

pnkfelix commented Oct 22, 2018

bors commented Oct 22, 2018

pnkfelix Oct 22, 2018

Choose a reason for hiding this comment

pnkfelix Oct 22, 2018

Choose a reason for hiding this comment

pnkfelix commented Oct 22, 2018

pnkfelix commented Oct 22, 2018

bors commented Oct 22, 2018

matthewjasper commented Oct 22, 2018 • edited Loading

Aaron1011 commented Oct 24, 2018

pnkfelix commented Oct 24, 2018

bors commented Oct 24, 2018

bors commented Oct 24, 2018

bors commented Oct 29, 2018

bors commented Oct 29, 2018

kennytm commented Oct 29, 2018

matthewjasper commented Oct 29, 2018

bors commented Oct 29, 2018

bors commented Oct 29, 2018

bors commented Oct 30, 2018

kennytm commented Oct 30, 2018

bors commented Oct 30, 2018

bors commented Oct 30, 2018

matthewjasper commented Oct 22, 2018 •

edited

Loading