rustdoc: mark unsafe fns in module page with superscript icons

[liigo]

Note: I'v changed the mark style. Now use superscript ⚠(U+26A0) (the old one is '[Unsafe]' literal).
Basically per https://botbot.me/mozilla/rust-docs/2016-10-19/?msg=75112017&page=1

New: mark with superscript icon

Old: mark with literal '[Unsafe]'

[rust-highfive]

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

[steveklabnik]

/cc @rust-lang/tools @rust-lang/docs

[sophiajt]

Hmm, I'd assume it's okay for standard library functions to be unsafe, otherwise they wouldn't be able to get very far.

Now, for functions in extern crates, that might be a handy thing to know.

[alexcrichton]

I personally feel like this is calling out unsafe functions too much. They're typically very well motivated to be part of a public API and don't necessarily need to have external pressure from rustdoc to remove them.

[GuillaumeGomez]

I dont like this change. The function is described as unsafe anyway so I don't see the need to add even more on that.

[nrc]

I assume the intention here is not unsafe-shaming, but informing users they will need an unsafe context to call from? That is useful info, but I think having it in the detail (rather than in this summary view) is fine (it is, I think less important to know at a glance than the function's type signature).

[GuillaumeGomez]

My point was that such an information shouldn't be in a description. I'm open to suggestions to make spotting unsafe functions easier but I don't like this approach.

[liigo]

I prefer safe functions to unsafe functions. I prefer safe way to unsafe way to do something. While searching for a way, I often explore corresponding module's page to find safe functions first, then unsafe ones for second best. Module pages already list functions' name and docs summary, adding an unsafety flag makes people easy to focus on both safe-only and unsafe-only functions, avoid wasting time click on undesired detail pages.

---

@nrc : I assume the intention here is not unsafe-shaming, but informing users they will need an unsafe context to call from?

Yes, of course.

@alexcrichton : They're typically very well motivated to be part of a public API and don't necessarily need to have external pressure from rustdoc to remove them.

Not leave a flag to remove them (but why you thought of this?). Just give users more information about unsafety before they open the detail page.

@GuillaumeGomez : The function is described as unsafe anyway so I don't see the need to add even more on that.

You must open its detail page to know it's unsafe or not. If people want to find a safe way to do something, they'd like not wasting time to click unsafe functions.

[frewsxcv]

Will bring this up at the docs meeting tomorrow.

[frewsxcv]

Brought this up at the meeting yesterday:

https://botbot.me/mozilla/rust-docs/2016-10-19/?msg=75112017&page=1

We didn't reach any decisions on this, but I think some people said they'd think about it.

---

My own opinion: I notice that (as per in the screenshot above) the "[Unsafe]" matches the visual styles of "[Deprecated]" and "[Unstable]". I think it's worth making a distinction between these items:

In the case of "[Deprecated]" and "[Unstable]", I think it makes sense to mention these because we really want to discourage the user from using these functions as the functions are phased out or subject to change.

In the case of "[Unsafe]", there are some very valid use cases (many for performance reasons) for using unsafe functions. I think we do enough at the compilation level for making the user aware of unsafe functions (though we could do more at the doc-comment level describing why it's unsafe).

I don't personally think it's necessary to treat both of these scenarios on the same level

[liigo]

I don't personally think it's necessary to treat both of these scenarios on the same level

On the same level because both are tags (or attributes) of a function. Tags itself are documentation. Not just stability and unsafety, we may add visibility here in the future too. (If rustdoc shows private items, visibility should be tagged.) I doubt there is exclusive scenario only for stability, but not for unsafety and visibility.

[liigo]

Note: I'v changed the mark style. Now we use superscript ⚠(U+26A0) (the old one is '[Unsafe]' literal), basically per https://botbot.me/mozilla/rust-docs/2016-10-19/?msg=75112017&page=1
cc @frewsxcv

[GuillaumeGomez]

Oh nice! I like this one.

[matthew-piziak]

That's quite elegant @liigo. 👍

[GuillaumeGomez]

cc @steveklabnik

[steveklabnik]

Seems like some more people like this. /cc @rust-lang/tools what do you think?

[frewsxcv]

I personally like the latest proposal more than the original one. I think I'm overall neutral on it though. I have some reservations about the use of a caution sign to represent 'unsafe', but I also can't think of a better symbol at the moment. I also am not a fan of 'alt' attributes in HTML as I feel they are unintuitive to trigger and don't work on mobile. That said, I unfortunately don't really have any concrete advice, but am interested to hear what others think.

[alexcrichton]

I agree with @frewsxcv in that I'm fairly neutral about this. I do like it better than [unsafe] as it's smaller and a not quite as in-your-face.

[liigo]

Unsafe function is unsafe, it's the truth. The unsafe keyword itself is a caution sign, which programmers pay more attention to. The new docs with caution signs just represent the code. Don't hesitate to tell it's unsafe, it is.

[jbcden]

I like this version too!

[steveklabnik]

It seems like people are neutral to mildly-positive about this change. I would merge, but since it's only a few days until we cut the next beta, I'd like to wait until just after, so we can get a full six weeks of feedback on nightly first.

[steveklabnik]

Beta has been branched. Let's see how the wider community feels.

@bors: r+

[bors]

📌 Commit 47515ad has been approved by steveklabnik

[Manishearth]

This should really be done on method names too.

[liigo]

@Manishearth Unsafe methods are already marked with unsafe keyword.

[Manishearth]

Yes, but it would be nice to be clearer about it.