Skip to content

Add comments explaining the unix command-line argument support. - #87279

Merged
bors merged 1 commit into
rust-lang:masterfrom
sunfishcode:document-unix-argv
Jul 21, 2021
Merged

Add comments explaining the unix command-line argument support.#87279
bors merged 1 commit into
rust-lang:masterfrom
sunfishcode:document-unix-argv

Conversation

@sunfishcode

Copy link
Copy Markdown
Member

Following up on #87236, add comments to the unix command-line argument
support explaining that the code doesn't mutate the system-provided
argc/argv, and that this is why the code doesn't need a lock or special
memory ordering.

r? @RalfJung

Following up on rust-lang#87236, add comments to the unix command-line argument
support explaining that the code doesn't mutate the system-provided
argc/argv, and that this is why the code doesn't need a lock or special
memory ordering.
@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2021
@sunfishcode

Copy link
Copy Markdown
MemberAuthor

Responding to the comment in #87236, release/acquire feels like it would obscure the intent here. This code is just deferring parsing for the system argc/argv, which is why relaxed is sufficient, but it's also the reason we don't need a mutex. If someone ever adds Rust code to mutate the argv array, they may need more than just release/acquire on argc/argv; they may need to reintroduce the mutex too. And they may need to think about potential interactions with C code which may also be able to observe the system-provided argv array (eg. if it's also registered in a .init_array function).

@RalfJung

Copy link
Copy Markdown
Member

Thanks! @bors r+ rollup

special memory ordering

I'd say Relaxed is the most special memory ordering, but we clearly have different perspectives here. ;)

@bors

bors commented Jul 19, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 2a56a68 has been approved by RalfJung

@borsbors 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 19, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jul 21, 2021
…alfJung
Add comments explaining the unix command-line argument support.
Following up on rust-lang#87236, add comments to the unix command-line argument
support explaining that the code doesn't mutate the system-provided
argc/argv, and that this is why the code doesn't need a lock or special
memory ordering.
r? `@RalfJung`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jul 21, 2021
…alfJung
Add comments explaining the unix command-line argument support.
Following up on rust-lang#87236, add comments to the unix command-line argument
support explaining that the code doesn't mutate the system-provided
argc/argv, and that this is why the code doesn't need a lock or special
memory ordering.
r? ``@RalfJung``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2021
…laumeGomez
Rollup of 9 pull requests
Successful merges:
- rust-lang#87187 (Fix NixOS detection)
- rust-lang#87206 (avoid temporary vectors/reuse iterators)
- rust-lang#87230 (Fix docblock <table> overflow)
- rust-lang#87273 (Recognize bounds on impls as const bounds)
- rust-lang#87279 (Add comments explaining the unix command-line argument support.)
- rust-lang#87301 (Fix typo in compile.rs)
- rust-lang#87311 (Get back the more precise suggestion spans of old regionck)
- rust-lang#87321 (Add long explanation for E0722)
- rust-lang#87342 (Add long explanation for E0757)
Failed merges:
- rust-lang#87270 (Don't display <table> in item summary)
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit eb54ddd into rust-lang:masterJul 21, 2021
@rustbotrustbot added this to the 1.55.0 milestone Jul 21, 2021
@sunfishcode
sunfishcode deleted the document-unix-argv branch July 21, 2021 16:41
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@sunfishcode@RalfJung@bors@rust-highfive@rustbot