Skip to content

Make AtomicBool the same size as bool - #33579

Merged
bors merged 1 commit into
rust-lang:masterfrom
Amanieu:atomic_bool2
May 14, 2016
Merged

Make AtomicBool the same size as bool#33579
bors merged 1 commit into
rust-lang:masterfrom
Amanieu:atomic_bool2

Conversation

@Amanieu

@AmanieuAmanieu commented May 12, 2016

Copy link
Copy Markdown
Member

Reopening #32365

This allows AtomicBool to be transmuted to a bool, which makes it more consistent with the other atomic types. Note that this now guarantees that the atomic type will always contain a valid bool value, which wasn't the case before (due to fetch_nand).

r? @alexcrichton

@Amanieu
Amanieuforce-pushed the atomic_bool2 branch 2 times, most recently from 4a69546 to 222b551CompareMay 12, 2016 16:12
@alexcrichton

alexcrichton commented May 13, 2016

Copy link
Copy Markdown
Member

@bors: r+ 915fa57

Thanks!

@alexcrichton

alexcrichton commented May 13, 2016

Copy link
Copy Markdown
Member

Note that the libs team discussed this change during triage the other day and the conclusion was that we're amenable to this with the other recent changes to std::sync::atomic

@bors

bors commented May 14, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 915fa57 with merge 8492b6a...

bors added a commit that referenced this pull request May 14, 2016
@bors
Auto merge of#33579- Amanieu:atomic_bool2, r=alexcrichton
8492b6a
Make AtomicBool the same size as bool
Reopening #32365
This allows `AtomicBool` to be transmuted to a `bool`, which makes it more consistent with the other atomic types. Note that this now guarantees that the atomic type will always contain a valid `bool` value, which wasn't the case before (due to `fetch_nand`).
r? @alexcrichton
@bors

bors commented May 14, 2016

Copy link
Copy Markdown
Collaborator

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

@bors
bors merged commit 915fa57 into rust-lang:masterMay 14, 2016
@strega-nil

strega-nil commented May 16, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor

@Amanieu Is this a guarantee? Like, on all platforms, all compilers, AtomicBool can be transmuted to bool? Or just specific platforms?

@Amanieu

Amanieu commented May 16, 2016

Copy link
Copy Markdown
MemberAuthor

As it is currently implemented, a transmute will work fine. However I am not sure whether we should make this guarantee to users. Note that currently, atomic-rs relies on AtomicU{8,16,32,64} being transmutable to & from their respective integer type.

@alexcrichton

alexcrichton commented May 16, 2016

Copy link
Copy Markdown
Member

When discussed with the libs team, the conclusion was that AtomicBool will not be able to be transmuted to bool, or rather it's not a guarantee that's provided.

@durkadurka mentioned this pull request May 18, 2016
Closed
@durka

durka commented May 18, 2016•
edited
Loading

Copy link
Copy Markdown
Contributor

This was a breaking change because people (edit: people such as @aturon) were previously transmuting usize to AtomicBool (see #33724).

@blussbluss added the relnotes Marks issues that should be documented in the release notes of the next release. label May 19, 2016
@mhristachemhristache mentioned this pull request May 26, 2016
Closed
@alexcrichtonalexcrichton mentioned this pull request Nov 26, 2018
Closed
@workingjubileeworkingjubilee mentioned this pull request Jul 27, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@alexcrichtonalexcrichton

Labels

relnotesMarks issues that should be documented in the release notes of the next release.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Amanieu@alexcrichton@bors@strega-nil@durka@bluss