Subpar documentation for ptr-to-ref conversion methods

[GrigorenkoPV]

While trying to merge #122492, some issues with the existing documentation became apparent.

The methods in question

• [ ] <*const T>::as_ref
• [ ] <*mut T>::as_ref
• [ ] <*mut T>::as_mut
• [ ] <*const T>::as_ref_unchecked [^unchecked]
• [ ] <*mut T>::as_ref_unchecked [^unchecked]
• [ ] <*mut T>::as_mut_unchecked [^unchecked]
• [ ] <*const T>::as_uninit_ref [^uninit]
• [ ] <*mut T>::as_uninit_ref [^uninit]
• [ ] <*mut T>::as_uninit_mut [^uninit]

[^unchecked]: Not yet stabilized (#122034) [^uninit]: Not yet stabilized (#75402)

The problems

• [ ] The overarching pain-point is that big parts of documentation are repeated for all (or at least the most) of the methods, which makes it difficult to keep the wording in sync.

  Documentation

• [ ] Generally subpar wording (probably due to these docs' being a patchwork of many separate changes). But also specifically:

• [ ] Imprecise wording around UnsafeCell

• [ ] ~Needlessly scary wording around the output's lifetime~ Or not?

  Doctests

• [x] ~Currently, they do printing instead of asserting. This should probably be changed to assertions.~ (Fixed in #126210)

• [ ] The assertions that we want should reflect the API being tested. (For example, we probably want an assertion that null pointer gets converted to None for as_ref, but no such assertion for as_ref_unchecked is possible.)

• [ ] Currently some asserts are hidden. Why?

  Moving forward

Feel free to voice your opinions/wishes/suggestions/questions regarding this issue or to submit PRs addressing any of the problems above (not necessarily all at once). Also, doctests can probably be worked on independently from the documentation itself.

Beep-Boop

@rustbot label +A-docs +A-doctests +C-discussion +C-enhancement +E-help-wanted

And now footnotes:

[lolbinarycat]

@rustbot claim

i'll tackle normalizing the doctests first, since the wording of the pointer methods has implications for unsafe code guidelines.

[GrigorenkoPV]

@rustbot reopen

[GrigorenkoPV]

Well, I'm powerless here. @rustbot knows how to close issues, but not how to reopen them. GitHub doesn't let me to reopen it myself. And @bors is very fond of marking issues as completed even when they aren't. #126210 addressed only one of the concerns (I've updated the OP to reflect this), but there's still a lot of work to be done.

@workingjubilee could you please help me in this and reopen the issue? Thanks!

[lolbinarycat]

whoops, yeah i should have said "part of" instead of "fixes", since my pr only addresses part of the issue, sorry about that (unfortunately i don't have the permissions to reopen issues either)

[lolbinarycat]

working on deduplicating documentation and it turns out that blurb also got copy-pasted onto several methods on NonNull, and those copies never got updated, so they're actually wrong, not listing the requirment of pointing to a valid value

[RalfJung]

Needlessly scary wording around the output's lifetime

I wouldn't say it is "needlessly" scary. The returned lifetime is equivalent to 'static. So this is as scary as creating a reference to a static mut, and we're currently taking steps to lint against that. This is a scary operation.

[workingjubilee]

My comment was less "it's needlessly scary" and more "it should be clear that if you do not specifically give a lifetime, it will infer any plausible lifetime from context".

This does include 'static! However, careful use in certain cases... and we should probably provide a demo of how... can achieve the desired lifetime. This usually is achieved by a helper fn that correctly models the lifetime constraints you wish to impose.

[RalfJung]

However, careful use in certain cases... and we should probably provide a demo of how... can achieve the desired lifetime.

Yes, just like a 'static lifetime can end up being subtyped to the desired lifetime.

The current signature of as_ref is for all intents and purposes equivalent to fn as_ref(*const T) -> &'static T. We should make that unambiguously clear.