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

Add an AtomicCell abstraction #62577

Merged
merged 1 commit into from
Jul 13, 2019
Merged

Add an AtomicCell abstraction #62577

merged 1 commit into from
Jul 13, 2019

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Jul 11, 2019

Split out from #61923.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2019
@@ -271,6 +322,8 @@ cfg_if! {

pub use std::sync::atomic::{AtomicBool, AtomicUsize, AtomicU32, AtomicU64};

pub use crossbeam_utils::atomic::AtomicCell;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why you define the struct AtomicCell above but then use it from the crossbeam_utils? Are they different? What is the purpose of including AtomicCell from crossbeam_utils here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, this code defines a unified set of structures related to multi-threadiness(?) with a caveat - the definitions are different for parallel_compiler cfg.

If you expand the omitted lines in the diff, you can see that the pub struct AtomicCell<T: Copy>(Cell<T>); (which is mainly a wrapper around Cell<T>, which isn't really atomic nor should it be in single-threaded case) is defined manually in the #[cfg(not(parallel_compiler))] branch.
Otherwise, if we build a parallel compiler, we just re-export crossbeam_utils::atomic::AtomicCell instead.

@matthewjasper
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2019

📌 Commit 498bdc9 has been approved by matthewjasper

@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 11, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 13, 2019
bors added a commit that referenced this pull request Jul 13, 2019
Rollup of 5 pull requests

Successful merges:

 - #62577 (Add an AtomicCell abstraction)
 - #62585 (Make struct_tail normalize when possible)
 - #62604 (Handle errors during error recovery gracefully)
 - #62636 (rustbuild: Improve assert about building tools once)
 - #62651 (Make some rustc macros more hygienic)

Failed merges:

r? @ghost
@bors bors merged commit 498bdc9 into rust-lang:master Jul 13, 2019
@jyn514 jyn514 mentioned this pull request Mar 29, 2021
6 tasks
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