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

Expand and organize offset_of! documentation. #138323

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Mar 10, 2025

  • Give example of how to get the offset of an unsized tail field (prompted by discussion Expand CloneToUninit documentation. #133055 (comment)).
  • Specify the return type.
  • Add section headings.
  • Reduce “Visibility is respected…”, to a single sentence.
  • Move offset_of_enum documentation to unstable book (with link to it).
  • Add offset_of_slice documentation in unstable book.

r? Mark-Simulacrum

* Give example of how to get the offset of an unsized tail field
  (prompted by discussion <rust-lang#133055 (comment)>).
* Specify the return type.
* Add section headings.
* Reduce “Visibility is respected…”, to a single sentence.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 10, 2025
Comment on lines 1259 to 1260
/// If the nightly-only feature `offset_of_enum` is enabled,
/// variants may be traversed as if they were fields.
/// `enum` variants may be traversed as if they were fields.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: maybe link to the tracking issue?

[`offset_of_enum`]: https://github.com/rust-lang/rust/issues/120141

But also, this is describing the interaction of this stable macro w/ an unstable feature. Maybe move this to a section like

# Interactions with unstable features

or maybe move this bit to docs for offset_of_enum? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK in this particular case to reference unstable features, even if we normally don't, since this seems like a natural place for people to look for that functionality. I don't think we have offset_of_enum! (yet?) - not sure what you mean by that.

Copy link
Member

@jieyouxu jieyouxu Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right.

I don't think we have offset_of_enum! (yet?) - not sure what you mean by that.

Not sure why I wrote this lmao.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed you meant to put the documentation in the unstable book, and I think that makes sense. Should I not?

(Also while writing that documentation, I found a bug-or-design-question: #138327.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I think that's what I wanted to say lol

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should at minimum link to those docs, even if we don't have a stable thing to link to - otherwise people will handroll stuff I suspect and we've seen that cause UB, etc. in the past.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IOW, probably do nothing here :D

Copy link
Contributor Author

@kpreid kpreid Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now moved all the unstable feature documentation to the unstable book, except for links to it from offset_of, and summaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants