Skip to content

update 'allocation' docs with memory atomicity requirement - #158586

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
asquared31415:ptr_alloc_atomic_docs
Jul 18, 2026
Merged

update 'allocation' docs with memory atomicity requirement#158586
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
asquared31415:ptr_alloc_atomic_docs

Conversation

@asquared31415

Copy link
Copy Markdown
Contributor

r? @RalfJung

As discussed on zulip

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 29, 2026
@rustbot

Copy link
Copy Markdown
Collaborator

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

@rustbot

Copy link
Copy Markdown
Collaborator

⚠️Warning⚠️

Comment threadlibrary/core/src/ptr/mod.rs Outdated
Comment on lines +124 to +125
//! primitives. If atomics are enabled for a target, all allocations in writeable memory must
//! support all atomic operations. The precise instructions used for atomic operations are

@RalfJungRalfJungJun 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
//! primitives. If atomics are enabled for a target, all allocations in writeable memory must
//! support all atomic operations. The precise instructions used for atomic operations are
//! primitives. Allocations must support all atomic operations that are available for a target (as determined by the `target_has_atomic*` cfg flags).
//! The precise instructions used for atomic operations are

Let's leave the discussion of immutable allocations to a future PR.

View changes since the review

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The module level docs for std::sync::atomic already specify that memory must be writeable for atomics, if that changes anything. https://doc.rust-lang.org/std/sync/atomic/#atomic-accesses-to-read-only-memory

I like the rephrasing though, will use that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The only way to get a non-writable Rust allocation is by impl Freeze statics (or promoted const).

It is still allowed to do atomic loads from this memory, AIUI, so all Rust allocations need to support being read with synchronization. For a write to this memory, that throws UB for writing without mutable provenance independent of the implementation memory supporting that write.

In other words, all allocations support atomic accesses. Using memory that cannot be written (atomically or otherwise) is a valid optimization when the memory is known to not be written to (e.g. when an attempted write would be UB).

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The docs I linked explicitly say that even loads may not always be valid on read-only memory. There are specific exceptions for specific target and sizes where the compiler guarantees it will generate loads suitable for read only memory.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's not just an optimization. The concept of an immutable allocation exists as a primitive in the AM, in my eyes. That's what the aforementioned atomic docs are implicitly referring to. We should update the "allocation" docs to make that explicit -- in a future PR.

@RalfJung

Copy link
Copy Markdown
Member

I view this as just a clarification so I don't think this needs FCP, but pinging @rust-lang/opsem @rust-lang/lang for your information.

@CAD97

Copy link
Copy Markdown
Contributor

I agree that this is just a clarification of preexisting requirements for Rust allocations to behave consistently.

@traviscrosstraviscross added T-lang Relevant to the language team T-opsem Relevant to the opsem team labels Jul 8, 2026
@RalfJung

Copy link
Copy Markdown
Member

I like the rephrasing though, will use that

@rustbot author

@rustbotrustbot 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 Jul 17, 2026
@rustbot

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rust-cloud-vms
rust-cloud-vmsBotforce-pushed the ptr_alloc_atomic_docs branch from 890d991 to d6f359bCompareJuly 17, 2026 18:36
@asquared31415

Copy link
Copy Markdown
ContributorAuthor

oops, this pr fell off the radar, ty for the reminder

@rustbot ready

@rustbotrustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2026
@RalfJung

Copy link
Copy Markdown
Member

Thanks!
@bors r+ rollup

@rust-bors

rust-borsBot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

📌 Commit d6f359b has been approved by RalfJung

It is now in the queue for this repository.

@rust-borsrust-borsBot 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 17, 2026
@jhprattjhpratt mentioned this pull request Jul 18, 2026
rust-borsBot pushed a commit that referenced this pull request Jul 18, 2026
Rollup of 5 pull requests
Successful merges:
- #159164 ([cg_ssa] Eliminate the `is_backend_{immediate,scalar_pair,ref}` methods)
- #159175 (add a fallback for more `f16` intrinsics )
- #158586 (update 'allocation' docs with memory atomicity requirement)
- #159390 (rustc: Avoid passing jobserver proxy around the compiler)
- #159468 (Android platform support: add links to make it easier to figure out the supported API levels)
@rust-bors
rust-borsBot merged commit ced9058 into rust-lang:mainJul 18, 2026
13 checks passed
@rustbotrustbot added this to the 1.99.0 milestone Jul 18, 2026
rust-timer added a commit that referenced this pull request Jul 18, 2026
Rollup merge of #158586 - asquared31415:ptr_alloc_atomic_docs, r=RalfJung
update 'allocation' docs with memory atomicity requirement
r? @RalfJung
As discussed [on zulip](https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/Region-based.20Atomic.20Capabilities/near/607225972)
@asquared31415
asquared31415 deleted the ptr_alloc_atomic_docs branch July 23, 2026 18:27
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-langRelevant to the language teamT-libsRelevant to the library team, which will review and decide on the PR/issue.T-opsemRelevant to the opsem team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@asquared31415@rustbot@RalfJung@CAD97@traviscross