update panicking() docs for panic=abort#153295
Conversation
There was a problem hiding this comment.
|
@SpriteOvO: 🔑 Insufficient privileges: not in review users |
|
@rustbot reroll |
library/std/src/thread/functions.rs
Outdated
| /// |-----------|----------------------------------------------------------------------| | ||
| /// | Linux | [clock_nanosleep] (Monotonic clock) | | ||
| /// | BSD except OpenBSD | [clock_nanosleep] (Monotonic Clock)] | | ||
| /// | BSD except OpenBSD | [clock_nanosleep] (Monotonic Clock) | |
There was a problem hiding this comment.
| /// | BSD except OpenBSD | [clock_nanosleep] (Monotonic Clock) | | |
| /// | BSD except OpenBSD | [clock_nanosleep] (Monotonic Clock) | |
Before merging, perhaps you'd like to fix this very very very minor nitpick 😇 -- aligning the table's divider.
There was a problem hiding this comment.
done and pushed.
9fd469d to
7072f83
Compare
library/std/src/thread/functions.rs
Outdated
| } | ||
|
|
||
| /// Determines whether the current thread is unwinding because of panic. | ||
| /// Determines whether the current thread panicking. |
There was a problem hiding this comment.
“panicking” -> “is panicking”
library/std/src/thread/functions.rs
Outdated
| /// | ||
| /// Note that this returns `true` when thread is unwinding due to panic, | ||
| /// as well as when a panic hook is executing and program is configured to abort | ||
| /// on panic (`panic=abort`) |
There was a problem hiding this comment.
Perhaps this could be rephrased as
… unwinding due to a panic or executing a panic hook. The latter case will still happen when
panic=abortis set.
Since this returns true during panic hooks even when unwinding, right?
Also grammar nits on the current version: “due to a panic”, “and the program”, and missing period at the end of the sentence.
There was a problem hiding this comment.
changed. also edited the underlying system calls table to match case and removed the trailing ] because it was sticking out to me
|
Reminder, once the PR becomes ready for a review, use |
b5e609d to
6ac34fc
Compare
|
@rustbot ready |
6ac34fc to
bf08176
Compare
library/std/src/thread/functions.rs
Outdated
| /// Note that this returns `true` both when the thread is unwinding due to a | ||
| /// panic or executing a panic hook. The latter case will still happen when |
There was a problem hiding this comment.
This returns
truewhen the thread is either unwinding due to a panic, or when it is executing a panic hook. Note that the latter case ...
Minor rewording suggestion to move the "note" bit to the second sentence since it's more of a note, and slightly smooth out the first sentence.
Please squash commits after this then LGTM, thanks for the additional fixes
There was a problem hiding this comment.
Done and commited. Sorry for the late work, I have an exam coming up.
There was a problem hiding this comment.
Replying within a day is not "late work", there is no rush in open source :) one more typo to fix then I'll approve
There was a problem hiding this comment.
Done, sorry about that. Also thankyou ;)
bf08176 to
d8330d0
Compare
library/std/src/thread/functions.rs
Outdated
| /// This returns `true` both when the thread is unwinding due to a panic. | ||
| /// or executing a panic hook. Note that the latter case will still happen |
There was a problem hiding this comment.
Should be a , not . after panic
rephrasing and grammar
d8330d0 to
014344b
Compare
…r=tgross35 update panicking() docs for panic=abort fixes rust-lang#151458 The documentation for `std::thread::panicking()` has not been changed since v1.0, even though panic hooks were added in v1.10. Current documentation is misleading for `panic=abort` `panicking()` can return `true` in 2 different cases: 1. Thread unwinds due to panic 2. Panic hook is executing with `panic=abort` r? @SpriteOvO
…uwer Rollup of 5 pull requests Successful merges: - #153280 (Add regression test for `doc(fake_variadic)` on reexports) - #153358 (Boundary tests for various Duration-float operations) - #153272 (Add `Path::absolute` method as alias for `std::path::absolute`) - #153295 (update panicking() docs for panic=abort) - #153352 (Migration of `LintDiagnostic` - part 6)
Rollup of 8 pull requests Successful merges: - #153280 (Add regression test for `doc(fake_variadic)` on reexports) - #153302 (x86: reserve `bl` and `bh` registers to match `rbx`) - #153358 (Boundary tests for various Duration-float operations) - #153048 (Improve irrefutable let-else lint wording) - #153258 (diag: Suppress `.clone()` suggestion inside derive macro expansions) - #153272 (Add `Path::absolute` method as alias for `std::path::absolute`) - #153295 (update panicking() docs for panic=abort) - #153352 (Migration of `LintDiagnostic` - part 6)
Rollup merge of #153295 - biscuitrescue:fix-panicking-docs, r=tgross35 update panicking() docs for panic=abort fixes #151458 The documentation for `std::thread::panicking()` has not been changed since v1.0, even though panic hooks were added in v1.10. Current documentation is misleading for `panic=abort` `panicking()` can return `true` in 2 different cases: 1. Thread unwinds due to panic 2. Panic hook is executing with `panic=abort` r? @SpriteOvO
fixes #151458
The documentation for
std::thread::panicking()has not been changed since v1.0, even though panic hooks were added in v1.10.Current documentation is misleading for
panic=abortpanicking()can returntruein 2 different cases:panic=abortr? @SpriteOvO