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

isolate non-mut static part from static-item section #760

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

king6cong
Copy link
Contributor

No description provided.

Comment on lines 16 to 17
that is not [interior mutable] may be placed in read-only memory. Static items
do not call [`drop`] at the end of the program.
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it could use some more re-arranging. The sentence about drop should probably be moved up since it applies to both mut and non-mut, correct?

Comment on lines +19 to +20
All access to a Non-`mut` static is safe, but there are a number of restrictions on
Non-`mut` statics:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not all these rules seem to apply to non-mut static. In fact, maybe only the first one?

Also, this sentence (the original) seems to be a very strong assertion (all access?). I'm not up to speed on safety issues, so maybe @Centril could validate what should be safe, or if this wording makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this diff seems to remove rules about static mut like the thing about referring to constants.

cc @RalfJung and @eddyb on the access thing, but notably, a static in an extern block is not safe to access and requires an unsafe context (https://doc.rust-lang.org/nightly/reference/items/external-blocks.html#statics). However, some static take be part of some internal contract that makes it unsound to access under some circumstances, even if the language rules do not enforce it. Under that reading it seems to me that the current wording makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

I mean you can access a non-mut static in safe code, so as a matter of fact this statement is correct, or at least some interpretation of it. ;) However, as a reflection of what the compiler is doing, extern statics should probably be excluded somewhere.

"all access" is slightly misleading though; writing to a static X: i32 is not safe and thus rejected by the compiler.

(This might also be a good place to call out that mutating a non-mut non-interior-mut extern static X: T is UB, even if that mutation happens by non-Rust code.)

that is not [interior mutable] may be placed in read-only memory. Static items
do not call [`drop`] at the end of the program.

All access to a static is safe, but there are a number of restrictions on
statics:
All access to a Non-`mut` static is safe, but there are a number of restrictions on
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
All access to a Non-`mut` static is safe, but there are a number of restrictions on
All access to a non-`mut` static is safe, but there are a number of restrictions on

All access to a static is safe, but there are a number of restrictions on
statics:
All access to a Non-`mut` static is safe, but there are a number of restrictions on
Non-`mut` statics:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Non-`mut` statics:
non-`mut` statics:

@ehuss ehuss added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Apr 7, 2020
@rustbot
Copy link
Collaborator

rustbot commented Dec 3, 2024

☔ The latest upstream changes (possibly bf115a4) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants