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

Revert PR #72389 - "Explain move errors that occur due to method calls involving self" #73594

Merged
merged 3 commits into from
Jun 23, 2020
Merged

Revert PR #72389 - "Explain move errors that occur due to method calls involving self" #73594

merged 3 commits into from
Jun 23, 2020

Conversation

Aaron1011
Copy link
Member

@Aaron1011 Aaron1011 commented Jun 21, 2020 •
edited by pnkfelix
Loading

reverts #72389

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2020
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+ p=1

@bors
Copy link
Contributor

bors commented Jun 21, 2020

📌 Commit 2959352 has been approved by petrochenkov

@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 Jun 21, 2020
@petrochenkov petrochenkov mentioned this pull request Jun 21, 2020
Closed
@RalfJung
Copy link
Member

(Would be nice when revert PR descriptions contain at least a link to where the explanation is for why a revert is needed.)

@Manishearth Manishearth mentioned this pull request Jun 21, 2020
Closed
@Manishearth
Copy link
Member

@bors p=5

This is blocking a whole chain of intra doc link fixes and stabilization

Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 21, 2020
@Manishearth
Rollup merge of rust-lang#73594 - Aaron1011:revert/move-fn-self-msg, …
8cabd20 
…r=petrochenkov

Revert PR rust-lang#72389 - "Explain move errors that occur due to method calls involving `self"

r? @petrochenkov
@Manishearth Manishearth mentioned this pull request Jun 21, 2020
Closed
@bors
Copy link
Contributor

bors commented Jun 22, 2020

⌛ Testing commit 2959352 with merge c0f4ee5d0e06d500b4cfd4bf19fcf4ee97707f89...

@Manishearth
Copy link
Member

My bad, this is blocking clippy test syncs, a different problem that is also high priority as right now clippy tests aren't being CId 😄

@bors
Copy link
Contributor

bors commented Jun 22, 2020

💔 Test failed - checks-azure

@bors 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 Jun 22, 2020
@petrochenkov petrochenkov 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 22, 2020
@Manishearth
Copy link
Member

So it's failing to compile the proc macro aux file. Unclear why. Feel free to temporarily remove the test and land this while we figure it out.

I was unable to get that failure to happen locally with an RTIM-rustc, might try full build tomorrow

@phansch phansch mentioned this pull request Jun 22, 2020
Merged
@flip1995
Copy link
Member

@Manishearth I'm able to reproduce the UI-test failure with ./x.py test src/tools/clippy. I haven't had time to look into this yet though.

Aaron1011 added 3 commits June 22, 2020 12:46
@Aaron1011
Revert "Rollup merge of rust-lang#72389 - Aaron1011:feature/move-fn-s…
ad9972a 
…elf-msg, r=nikomatsakis"

This reverts commit 372cb9b, reversing
changes made to 5c61a8d.
@Aaron1011
Re-enable Clippy tests
d3feb8b
@Aaron1011
Stop using old version of syn in rustc-workspace-hack
e2ab98d 
None of the tools seem to need syn 0.15.35, so we can just build syn
1.0.

This was causing an issue with clippy's `compile-test` program: since
multiple versions of `syn` would exist in the build directory, we would
non-deterministically pick one based on filesystem iteration order. If
the pre-1.0 version of `syn` was picked, a strange build error would
occur (see
rust-lang#73594 (comment))

To prevent this kind of issue from happening again, we now panic if we
find multiple versions of a crate in the build directly, instead of
silently picking the first version we find.
@Aaron1011
Copy link
Member Author

Build error: https://gist.github.com/Aaron1011/479c460250ed9ead4e0e6631a2535071

@Aaron1011 Aaron1011 force-pushed the revert/move-fn-self-msg branch from 2959352 to e2ab98d Compare June 22, 2020 17:32
@Aaron1011
Copy link
Member Author

@Manishearth @flip1995: I found the cause of the error - we had multiple versions of syn in the build directory that Clippy manually searched. we were non-deterministically picking the first one we encountered when iterating over the filesystem, which lead to the build error I posted above.

This should be ready to merge.

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 22, 2020

📌 Commit e2ab98d has been approved by Manishearth

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2020
@bors
Copy link
Contributor

bors commented Jun 22, 2020

⌛ Testing commit e2ab98d with merge cbf356a...

@bors
Copy link
Contributor

bors commented Jun 23, 2020

☀️ Test successful - checks-azure
Approved by: Manishearth
Pushing cbf356a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 23, 2020
@bors bors merged commit cbf356a into rust-lang:master Jun 23, 2020
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 23, 2020
@Aaron1011
Stop using old version of syn in rustc-workspace-hack
e11b873 
None of the tools seem to need syn 0.15.35, so we can just build syn
1.0.

This was causing an issue with clippy's `compile-test` program: since
multiple versions of `syn` would exist in the build directory, we would
non-deterministically pick one based on filesystem iteration order. If
the pre-1.0 version of `syn` was picked, a strange build error would
occur (see
rust-lang/rust#73594 (comment))

To prevent this kind of issue from happening again, we now panic if we
find multiple versions of a crate in the build directly, instead of
silently picking the first version we find.
This was referenced Jun 23, 2020
Merged
Merged
Closed
@Aaron1011 Aaron1011 mentioned this pull request Jun 23, 2020
Closed
Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Jun 24, 2020
@Aaron1011
Explain move errors that occur due to method calls involving self
cf4c2db 
This is a re-attempt of rust-lang#72389 (which was reverted in rust-lang#73594)
Instead of using `ExpnKind::Desugaring` to represent operators, this PR
checks the lang item directly.
@Aaron1011 Aaron1011 mentioned this pull request Jun 24, 2020
Merged
Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Jun 26, 2020
@Aaron1011
Explain move errors that occur due to method calls involving self
fa6a61c 
This is a re-attempt of rust-lang#72389 (which was reverted in rust-lang#73594)
Instead of using `ExpnKind::Desugaring` to represent operators, this PR
checks the lang item directly.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 28, 2020
@Manishearth
Rollup merge of rust-lang#73708 - Aaron1011:feature/reland-move-fn-se…
b236e49 
…lf-msg, r=davidtwco

Explain move errors that occur due to method calls involving `self` (take two)

This is a re-attempt of rust-lang#72389 (which was reverted in rust-lang#73594)
Instead of using `ExpnKind::Desugaring` to represent operators, this PR
checks the lang item directly.
@nnethercote
Copy link
Contributor

This was a perf win of up to 3.1% on a few benchmarks.

@nnethercote nnethercote changed the title Revert PR #72389 - "Explain move errors that occur due to method calls involving `self" Revert PR #72389 - "Explain move errors that occur due to method calls involving self" Jun 29, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.46
Development

Successfully merging this pull request may close these issues.
9 participants
@Aaron1011 @petrochenkov @bors @RalfJung @Manishearth @flip1995 @nnethercote @cuviper @rust-highfive