Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subpar documentation for ptr-to-ref conversion methods #124669

Open
1 of 16 tasks
GrigorenkoPV opened this issue May 3, 2024 · 8 comments
Open
1 of 16 tasks

Subpar documentation for ptr-to-ref conversion methods #124669

GrigorenkoPV opened this issue May 3, 2024 · 8 comments
Assignees
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-doctests Area: Documentation tests, run by rustdoc C-discussion Category: Discussion or questions that doesn't represent real issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@GrigorenkoPV
Copy link
Contributor

GrigorenkoPV commented May 3, 2024

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 1
  • <*mut T>::as_ref_unchecked 1
  • <*mut T>::as_mut_unchecked 1
  • <*const T>::as_uninit_ref 2
  • <*mut T>::as_uninit_ref 2
  • <*mut T>::as_uninit_mut 2

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

Doctests

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:

Footnotes

  1. Not yet stabilized (Tracking Issue for raw-pointer-to-reference conversion methods #122034) 2 3

  2. Not yet stabilized (Tracking Issue for pointer methods returning MaybeUninit<T> #75402) 2 3

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-doctests Area: Documentation tests, run by rustdoc C-discussion Category: Discussion or questions that doesn't represent real issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. labels May 3, 2024
@saethlin saethlin added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 11, 2024
@lolbinarycat
Copy link
Contributor

@rustbot claim

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

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 12, 2024
…=workingjubilee

docs(core): make more const_ptr doctests assert instead of printing

improves on rust-lang#124669
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 12, 2024
…=workingjubilee

docs(core): make more const_ptr doctests assert instead of printing

improves on rust-lang#124669
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jun 12, 2024
…=workingjubilee

docs(core): make more const_ptr doctests assert instead of printing

improves on rust-lang#124669
@bors bors closed this as completed in fe52b54 Jun 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 12, 2024
Rollup merge of rust-lang#126210 - lolbinarycat:ptr_doctest_assert, r=workingjubilee

docs(core): make more const_ptr doctests assert instead of printing

improves on rust-lang#124669
@GrigorenkoPV
Copy link
Contributor Author

@rustbot reopen

@GrigorenkoPV
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

workingjubilee commented Aug 24, 2024

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
Copy link
Member

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.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 26, 2024
…=cuviper

deduplicate and clarify rules for converting pointers to references

part of rust-lang#124669
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 26, 2024
…=cuviper

deduplicate and clarify rules for converting pointers to references

part of rust-lang#124669
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 27, 2024
Rollup merge of rust-lang#128157 - lolbinarycat:unify-ptr-ref-docs, r=cuviper

deduplicate and clarify rules for converting pointers to references

part of rust-lang#124669
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-doctests Area: Documentation tests, run by rustdoc C-discussion Category: Discussion or questions that doesn't represent real issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-help-wanted Call for participation: Help is requested to fix this issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
Development

No branches or pull requests

6 participants