std: Make atomics immutable. #11583 #12430

brson · 2014-02-21T01:11:01Z

No description provided.

alexcrichton · 2014-02-21T01:16:42Z

Could you add some rationale to the commit message as well? I'm not entirely convinced just yet, but I'm getting to the point where I could be persuaded.

brson · 2014-02-21T01:24:22Z

@alexcrichton Done.

brson · 2014-02-21T01:32:11Z

The word 'immutable' is becoming increasingly incorrect.

alexcrichton · 2014-02-24T07:16:50Z

I'm curious, did you get an email saying that the travis build failed? I haven't been getting emails...

brson · 2014-02-24T07:28:18Z

No
On Feb 23, 2014 11:17 PM, "Alex Crichton" notifications@github.com wrote:

I'm curious, did you get an email saying that the travis build failed? I
haven't been getting emails...

Reply to this email directly or view it on GitHubhttps://github.com//pull/12430#issuecomment-35862574
.

alexcrichton · 2014-02-24T07:29:26Z

Ah, it appears that there's an issue open about this: travis-ci/travis-ci#1417. I suppose we'll just have to be vigilant for now.

brson · 2014-02-25T01:15:16Z

Going to add a few more commits to fix additional type system violations with atomics.

alexcrichton · 2014-02-25T03:12:30Z

All the commits look fine other than the one I commented on, and I'm just curious about the clarification there. I'm getting more perturbed that we're saying & == immutable as well because it's kinda just becoming "an aliasable pointer"

brson · 2014-02-25T17:35:02Z

@alexcrichton Indeed I believe it's accurate to say that nearly the only thing that & means on its own is "an aliasable pointer", plus that it is threadsafe. What & actually guarantees depends on what it's pointing to: if it points to something Freeze then it's really pointing to immutable memory. These atomics are generic over an unbounded T, and in practice is only used on primitive types that are freeze, meaning that if they take a &T then mutate them, then they are violating the promises made by the type system.

alexcrichton · 2014-02-26T18:27:14Z

I have been convinced, r=me.

It appears travis has some problems with this, though.

In Rust, the strongest guarantee that `&mut` provides is that the memory pointed to is *not aliased*, whereas `&`'s guarantees are much weaker: that the value can be aliased, and may be mutated under proper precautions (interior mutability). Our atomics though use `&mut` for mutation even while creating multiple aliases, so this changes them to use 'interior mutability', mutating through immutable references.

These mutate values behind references that are Freeze, which is not allowed.

I'm not comfortable exposing public functions that purport to do atomic operations on arbitrary T.

Support for this is less universal than for word-size things; it has no users; i'd rather play it safe.

flaper87 · 2014-02-28T09:52:16Z

src/libstd/sync/atomics.rs

-pub static INIT_ATOMIC_INT  : AtomicInt  = AtomicInt  { v: 0, nopod: marker::NoPod };
-pub static INIT_ATOMIC_UINT : AtomicUint = AtomicUint { v: 0, nopod: marker::NoPod };
-pub static INIT_ATOMIC_U64 : AtomicU64 = AtomicU64 { v: 0, nopod: marker::NoPod };
+pub static INIT_ATOMIC_FLAG : AtomicFlag = AtomicFlag {


The new static constrains will complain about this, unless they're made mutable. Immutable static items values must be Freeze

I see this as quite unfortunate, sadly. These are all just initializer patterns, none of them are meant to be used. Requiring them to be mut means that you could in theory write INIT_ATOMIC_UINT.fetch_add(1) which seems... bad.

@brson, I suppose for now these should be static mut. cc @nikomatsakis, this is an unforseen case in statics that I didn't think of earlier sadly.

Why not just make them functions? You're really asking for rvalues, that's what a function is for.

These are all static initializers (for other static atomics), so they can't be functions sadly.

Ah, yes, I see. It seems we need some kind of static rvalue to cover this case. constexpr ftw.

I guess this also falls down into the re-definition of what Freeze actually means (ref).

I think our best way out of this issue is to make them static mut for now (pls, add a FIXME)

This seems to relate to the "dual role" -- a static both represents a constant value and a static bit of memory. It'd be nice if we could distinguish the two -- we could permit "constant values" to be non-freeze, since you wouldn't be able to take their address.

ps the repetition of markers here would be so much nicer under the opt-in approach ;)

alexcrichton · 2014-03-20T17:16:12Z

Closing in favor of #13036

Closes #11583, rebasing of #12430 now that we've got `Share` and better analysis with statics.

minor: Remove unused code

…exendoo Fix dependency specifications As Clippy lacks `Cargo.lock`, it makes sense to test its dependency specifications with [direct-minimal-versions](https://doc.rust-lang.org/cargo/reference/unstable.html#direct-minimal-versions). This can be done with the following addition to `.cargo/config.toml`. ```toml [unstable] direct-minimal-versions = true ``` * `tempfile` 3.3 is required by `clippy_lints`. * `regex` 1.5.5 is required by `ui_test` 0.22.2. * `quote` 1.0.25 is required by `syn` 2.0.0. * `serde` 1.0.145 is required by `toml` 0.7.3. changelog: none

brson added 4 commits February 27, 2014 12:21

std: Make the generic atomics take unsafe pointers

df07e7a

These mutate values behind references that are Freeze, which is not allowed.

std: Make the generic atomics in sync::atomics private

bcfff9d

I'm not comfortable exposing public functions that purport to do atomic operations on arbitrary T.

std: Remove AtomicU64

b2b469a

Support for this is less universal than for word-size things; it has no users; i'd rather play it safe.

alexcrichton mentioned this pull request Feb 28, 2014

Rolling up some more PRs #12614

Merged

flaper87 reviewed Feb 28, 2014
View reviewed changes

This was referenced Mar 14, 2014

fix MIPS target #12878

Closed

Immutable atomics #13036

Merged

alexcrichton closed this Mar 20, 2014

bors added a commit that referenced this pull request Mar 22, 2014

auto merge of #13036 : alexcrichton/rust/atomics, r=alexcrichton

069cede

Closes #11583, rebasing of #12430 now that we've got `Share` and better analysis with statics.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022

Auto merge of rust-lang#12430 - Veykril:completions, r=Veykril

e25cbf8

minor: Remove unused code

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

std: Make atomics immutable. #11583 #12430

std: Make atomics immutable. #11583 #12430

brson commented Feb 21, 2014

alexcrichton commented Feb 21, 2014

brson commented Feb 21, 2014

brson commented Feb 21, 2014

alexcrichton commented Feb 24, 2014

brson commented Feb 24, 2014

alexcrichton commented Feb 24, 2014

brson commented Feb 25, 2014

alexcrichton commented Feb 25, 2014

brson commented Feb 25, 2014

alexcrichton commented Feb 26, 2014

flaper87 Feb 28, 2014

alexcrichton Feb 28, 2014

nikomatsakis Feb 28, 2014

alexcrichton Feb 28, 2014

nikomatsakis Feb 28, 2014

flaper87 Feb 28, 2014

nikomatsakis Feb 28, 2014

nikomatsakis Feb 28, 2014

alexcrichton commented Mar 20, 2014

std: Make atomics immutable. #11583 #12430

std: Make atomics immutable. #11583 #12430

Conversation

brson commented Feb 21, 2014

alexcrichton commented Feb 21, 2014

brson commented Feb 21, 2014

brson commented Feb 21, 2014

alexcrichton commented Feb 24, 2014

brson commented Feb 24, 2014

alexcrichton commented Feb 24, 2014

brson commented Feb 25, 2014

alexcrichton commented Feb 25, 2014

brson commented Feb 25, 2014

alexcrichton commented Feb 26, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Mar 20, 2014