Unix ExitStatus comments and a tiny docs fix #90704
Conversation
|
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
library/std/src/process.rs
Outdated
| // We speak slightly loosely (here and in various other places in the stdlib docs) about `exit` | ||
| // rather than `_exit`. Strictly speaking, an exit status is a value passed to the `_exit` system | ||
| // call, around which `exit` is a library wrapper. But anyone who knows the difference will not be | ||
| // misled; anyone who does not, does not want to be distracted by some subtle point of UNIX APIs. | ||
| // (Also, the distinction between syscalls and library functions is not standardised, although Unix | ||
| // people typically have common notions.) |
There was a problem hiding this comment.
This comes across to me very much like "UNIX people understand this and other people don't need to" elitism. I'm not suggesting that that's the intent, but I feel like it's the net effect.
I also don't think it's necessary for the purposes of conveying this point. As far as I can tell, libc::exit and libc::_exit and the Linux syscall exit and return from main all accept an exit status value, and pass it along. So the point worth conveying here seems more like this:
| // We speak slightly loosely (here and in various other places in the stdlib docs) about `exit` | |
| // rather than `_exit`. Strictly speaking, an exit status is a value passed to the `_exit` system | |
| // call, around which `exit` is a library wrapper. But anyone who knows the difference will not be | |
| // misled; anyone who does not, does not want to be distracted by some subtle point of UNIX APIs. | |
| // (Also, the distinction between syscalls and library functions is not standardised, although Unix | |
| // people typically have common notions.) | |
| // When this documentation refers to a status passed to `exit` or similar, this | |
| // loosely refers to any mechanism that exits a process and accepts an exit | |
| // code, including system calls, library calls that make such system calls, or | |
| // returning from a program's main function to a library that invokes such | |
| // system calls. |
(modulo word-wrapping)
There was a problem hiding this comment.
I think you have missed something: This isn't a doc comment. I agree that this kind of language would be inappropriate in public docs.
The audience is other people who are editing this code or its documentation. The purpose is to explain to any passing Unix pedants why the docs are the way they are.
There was a problem hiding this comment.
I didn't think it was a doc comment. It's still documentation, just with an audience of people who read the code. We're friendly to each other, too, not just to users.
That said, I do understand the desire to quell pedantry, UNIXy or otherwise. But I think if we want to avoid attracting or encouraging that kind of UNIX pedantry, it'll also help to avoid playing into divisions based on who has which bits of UNIX ingroup knowledge.
apiraino
added
the
T-libs
As discussed here rust-lang#88300 (comment) exit is (conventionally) a library function, with _exit being the actual system call. I have checked the other references and they say "if the process terminated by calling `exti`". I think despite the slight imprecision (strictly, it should read iff ... `_exit`), this is clearer. Anyone who knows about the distinction between `exit` and `_exit` will not be confused. `_exit` is the correct traditional name for the system call, despite Linux calling it `exit_group` or `exit`: https://www.freebsd.org/cgi/man.cgi?query=_exit&sektion=2&n=1 Signed-off-by: Ian Jackson <[email protected]>
With cross-reference. Signed-off-by: Ian Jackson <[email protected]>
As discussed here rust-lang#88300 (comment) I felt this was the best place to put this (rather than next to ExitStatusExt). After all, it's a property of the ExitStatus type on Unix. Signed-off-by: Ian Jackson <[email protected]>
ijackson
force-pushed
the
exitstatus-comments
branch
from
0863e33
to
fe39fb3
Compare
|
I have (force pushed and) updated the wording. I hope this now doesn't read as patronising. |
|
@bors r+ Thank you for the documentation, and for the adaptations as well. |
|
📌 Commit fe39fb3 has been approved by |
bors
added
S-waiting-on-bors
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#90589 (rustc_llvm: update PassWrapper for recent LLVM) - rust-lang#90644 (Extend the const swap feature) - rust-lang#90704 (Unix ExitStatus comments and a tiny docs fix) - rust-lang#90761 (Shorten Span of unused macro lints) - rust-lang#90795 (Add more comments to explain the code to generate the search index) - rust-lang#90798 (Document `unreachable!` custom panic message) - rust-lang#90826 (rustc_feature: Convert `BuiltinAttribute` from tuple to a struct) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Some nits left over from #88300