doc: panic::{take_hook, set_hook, update_hook} abort when called from…

[sftse]

We had an abort in production because I skimmed the docs for take_hook, not realizing that calling it from a panicking thread results in an abort. The code was written defensively to account for panics but failed to account for a function that outright aborts if called under the wrong circumstances.

It seems better to short circuit the comment and cut straight to the chase instead of letting reader deduce 2*panic == abort.

OTOH, there is no precedent for an Aborts section anywhere else in std.

[rustbot]

r? @ChrisDenton

rustbot has assigned @ChrisDenton.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[ChrisDenton]

I think it's a good idea to note that this may abort but I don't think this is quite the right way to document it.

The current documentation is accurate in the sense that it is a panic. However, not mentioned is a panic within a panic will cause an abort. This is not quite the same as promising an abort though. Panics can be caught whereas aborts can't be.

[sftse]

Would this documentation be considered normative?

Afaik there currently is no way to catch this panic from user code, but if that changes we could update the docs again.
If we can't observe the panic the current docs feel like an implementation detail of little relevance to callers.

The discussions around changing allocation failures from abort to panic seem to indicate such changes are not considered breaking, so the door doesn't seem shut to change this in the future.

[ChrisDenton]

An example of it being caught: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=989868e4e9523a3d8d9a06814f0454a0

More generally we do have to be very careful when making promises in standard library documentation. We can however make weaker statements about current or probable behaviour so long as there are appropriate caveats. For example, for I/O we document some implementation details but also make clear that this is purely informative and may change in the future: https://doc.rust-lang.org/stable/std/io/index.html#platform-specific-behavior

[sftse]

That's already quite informative and clears up a misconception I had.

However, not mentioned is a panic within a panic will cause an abort.

Fuzzy pattern matching led me to believe std::thread::panicking() is a reliable definition of there currently being a panic in progress.

My proposed doc change is clearly incorrect in that case.