Highlight when APIs panic in rustdoc

d2cd5f4
Opened by Manish Goregaokar at 2018-05-15 21:46:13

When an API specifically has a "will panic on X condition" guarantee (e.g. indexing APIs, unwrap, expect, perhaps allocating APIs) it would be nice if the stdlib could highlight these the way we do unsafe methods.

https://twitter.com/0x424c41434b/status/944316751919140865

Worth prototyping this behind a feature flag for now, and then discussing whether we should be stabilizing it (or keeping it forever unstable for the stdlib).

cc @rust-lang/docs

(I'm mentoring 0x424c41434b on implementing this, even if we eventually decide not to do this)

  1. The relevant code is probably in doc_impl_item, see where we call document_stability to do something similar for stability messages.

    You should be able to find the attribute on the clean::Item https://michael-f-bryan.github.io/rustc-internal-docs/rustdoc/clean/struct.Item.html

    Manish Goregaokar at 2017-12-23 03:53:41

  2. We already have a strong convention -- panics should be mentioned in the doc. In rust-lang/rust, under the # Panics heading (example). One part of this issue is to increase coverage? To make sure even the "obvious" cases are covered, for example the trait method Index::index wherever implemented with panicking bounds checks.

    How does a label relate to the already present documentation?

    Also, the twitter thread has some misconceptions, like Vec::new and String::new can panic (and they can't). And that panics are only catchable at thread boundaries (but we have catch_unwind).

    bluss at 2017-12-23 12:46:26

  3. The label is what we should turn the present documentation convention into.

    I don't think the issues with the thread are relevant. For vec::new there are proposals for oom=panic which I thought had landed, and I corrected the thing about catch_unwind.

    Manish Goregaokar at 2017-12-23 13:01:53

  4. @Manishearth: Can you write it on the roadmap of the doc team so we can discuss about it in our next meeting (which will be in 2 weeks) please?

    Guillaume Gomez at 2017-12-24 15:41:01

  5. @GuillaumeGomez https://github.com/rust-lang/rust-roadmap/issues/20 ? It doesn't seem like folks leave todo items on that issue. Could you mention this in the appropriate place?

    Manish Goregaokar at 2017-12-25 02:39:27

  6. just added this to the docs team meeting agenda

    Corey Farwell at 2017-12-25 02:48:24

  7. @manishearth Vec::new/String::new does not allocate, and therefore, oom could never make it panic.

    On Sun, Dec 24, 2017 at 9:48 PM, Corey Farwell notifications@github.com wrote:

    just added this to the docs team meeting agenda https://public.etherpad-mozilla.org/p/rust-docs

    — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/46963#issuecomment-353821073, or mute the thread https://github.com/notifications/unsubscribe-auth/AABsigY45WdwD4n4dbn_GWx9Kp1opYiaks5tDwz6gaJpZM4RLkDR .

    Steve Klabnik at 2017-12-25 16:40:04

  8. I'm aware; there's a proposal to change this with oom=panic.

    On Dec 25, 2017 10:10 PM, "Steve Klabnik" notifications@github.com wrote:

    @manishearth Vec::new/String::new does not allocate, and therefore, oom could never make it panic.

    On Sun, Dec 24, 2017 at 9:48 PM, Corey Farwell notifications@github.com wrote:

    just added this to the docs team meeting agenda https://public.etherpad-mozilla.org/p/rust-docs

    — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/46963#issuecomment-353821073, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABsigY45WdwD4n4dbn_ GWx9Kp1opYiaks5tDwz6gaJpZM4RLkDR> .

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/46963#issuecomment-353879432, or mute the thread https://github.com/notifications/unsubscribe-auth/ABivSJA7eqE_Pl2oZ-VPsiTD_GT3E2ofks5tD8_3gaJpZM4RLkDR .

    Manish Goregaokar at 2017-12-25 23:27:02

  9. How is oom=panic related to the capacity used with Vec::new and String::new?

    Steven Fackler at 2017-12-25 23:34:37

  10. Because with_capacity can trigger a panicky oom in that case?

    On Dec 26, 2017 5:05 AM, "Steven Fackler" notifications@github.com wrote:

    How is oom=panic related to the capacity used with Vec::new and String::new?

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/46963#issuecomment-353895743, or mute the thread https://github.com/notifications/unsubscribe-auth/ABivSOm21zPpOORod69ZaJozwkfEgnrHks5tEDEkgaJpZM4RLkDR .

    Manish Goregaokar at 2017-12-25 23:35:41

  11. new doesn't call with_capacity: https://github.com/rust-lang/rust/blob/master/src/liballoc/vec.rs#L324-L329

    Steven Fackler at 2017-12-25 23:42:15

  12. Oh, sorry. Meant push/with_capacity, my bad.

    On Dec 26, 2017 5:12 AM, "Steven Fackler" notifications@github.com wrote:

    new doesn't call with_capacity: https://github.com/rust-lang/ rust/blob/master/src/liballoc/vec.rs#L324-L329

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/46963#issuecomment-353895966, or mute the thread https://github.com/notifications/unsubscribe-auth/ABivSKhZbU3XrJuKJG5BGihPfb0qtgPCks5tEDLsgaJpZM4RLkDR .

    Manish Goregaokar at 2017-12-25 23:46:27

  13. We talked about this a little at the docs team meeting today. Overall, we think it could be a good thing, but we'd like to see an initial implementation before casting final judgement. If you're still interested, feel free to sketch out something.

    QuietMisdreavus at 2018-01-02 20:59:22

  14. Hello,

    Sorry kind of nervous, first comment on git. I thought this would be great and I would like to work on it if that is okay

    UnsafeCell<MaybeUninit<Anxiety<T>>> at 2018-01-05 14:55:24

  15. Sure! My second comment has some info on how to get started with this, let me know (here or on Twitter) if you have questions!

    Manish Goregaokar at 2018-01-05 15:00:37

  16. It's also worth adding whatever processing to the document function so that it can end up on bare function pages too. (Bare functions are rendered in item_function, which defers its stability/markdown printing to document.)

    Otherwise, feel free to work on it! If you have any questions and @Manishearth isn't available, feel free to ask me! I love to help people work on rustdoc.

    QuietMisdreavus at 2018-01-05 15:19:11

  17. @BitExplosion hey, have you had a chance to look into this yet?

    Manish Goregaokar at 2018-02-08 14:54:53