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

Remove unused code from rustc_data_structures::sync #83653

Merged
merged 2 commits into from
Jun 5, 2021
Merged

Remove unused code from rustc_data_structures::sync #83653

merged 2 commits into from
Jun 5, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 29, 2021

Found using https://github.com/est31/warnalyzer. Follow-up to #83185.

r? @Zoxc cc @oli-obk

@jyn514 jyn514 added 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 Mar 29, 2021
@bjorn3
Copy link
Member

bjorn3 commented Mar 31, 2021

AtomicCell is used by neither non-parallel, nor parallel rustc, so I think it is fine to remove it. Removing all the unused methods from the non-parallel versions is a bad idea I think as the parallel do still have them. I think the non-parallel and parallel versions of all types should have an identical api surface to prevent confusion.

@jyn514 jyn514 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2021
@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2021
@jyn514
Copy link
Member Author

jyn514 commented May 19, 2021

Hmm, not sure who a good reviewer would be - maybe r? @wesleywiser ?

@wesleywiser
Copy link
Member

There's a comment here that should be adjusted since we're removing AtomicCell

rust/compiler/rustc_data_structures/src/sync.rs

Line 87 in 35bab92

/// It differs from `AtomicCell` in that it has explicit ordering arguments

r=me after that.

@bors rollup

jyn514 added 2 commits June 4, 2021 15:25
@jyn514
Remove unused code from rustc_data_structures::sync
1a5cc25 
Found using https://github.com/est31/warnalyzer.
@jyn514
Unify parallel and non-parallel APIs
3412957 
It's confusing for these to be different, even if some of the methods
are unused.
@jyn514 jyn514 force-pushed the unused-sync-code branch from e93e4d0 to 3412957 Compare June 4, 2021 19:26
@jyn514
Copy link
Member Author

jyn514 commented Jun 4, 2021

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Jun 4, 2021

📌 Commit 3412957 has been approved by wesleywiser

@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 4, 2021
@JohnTitor JohnTitor mentioned this pull request Jun 4, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2021
@bors
Auto merge of rust-lang#86006 - JohnTitor:rollup-97iuoi3, r=JohnTitor
704934d 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#83653 (Remove unused code from `rustc_data_structures::sync`)
 - rust-lang#84466 (rustdoc: Remove `PrimitiveType::{to_url_str, as_str}`)
 - rust-lang#84880 (Make match in `register_res` easier to read)
 - rust-lang#84942 (rustdoc: link to stable/beta docs consistently in documentation)
 - rust-lang#85853 (Warn against boxed DST in `improper_ctypes_definitions` lint)
 - rust-lang#85939 (Fix suggestion for removing &mut from &mut macro!().)
 - rust-lang#85966 (wasm: Make simd types passed via indirection again)
 - rust-lang#85979 (don't suggest unsized indirection in where-clauses)
 - rust-lang#85983 (Update to semver 1.0.3)
 - rust-lang#85988 (Note that `ninja = false` goes under `[llvm]`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7564312 into rust-lang:master Jun 5, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 5, 2021
@jyn514 jyn514 deleted the unused-sync-code branch June 5, 2021 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
7 participants
@jyn514 @bjorn3 @wesleywiser @bors @crlf0710 @JohnCSimon @rustbot