Skip to content

Use intra-doc links in mem::manually_drop & mem::maybe_uninit #75214

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 15, 2020
Merged

Use intra-doc links in mem::manually_drop & mem::maybe_uninit #75214

merged 1 commit into from
Aug 15, 2020

Conversation

@chansuke
Copy link
Contributor

@chansuke chansuke commented Aug 6, 2020 •
edited by jyn514
Loading

This is partial fixes for #75080.

@rust-highfive
Copy link
Contributor

r? @cramertj

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 6, 2020
@jyn514 jyn514 assigned jyn514 and unassigned cramertj Aug 6, 2020
@jyn514 jyn514 added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Aug 6, 2020
jyn514
jyn514 reviewed Aug 6, 2020
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Looks good once links are fixed and CI is passing :)

library/core/src/mem/manually_drop.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Rustdoc will not resolve this <T>: #62834. But also I think it was meant to link to mem::zeroed, not MaybeUnint::zeroed, so you can leave it out altogether and it will still be resolved.

Suggested change
/// [`mem::zeroed`]: MaybeUninit::<T>::zeroed
/// [`MaybeUninit<T>`]: MaybeUninit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your suggestion.

/// [MaybeUninit]: MaybeUninit shows unresolved link error...

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 fixed it with [MaybeUninit]: crate::mem::MaybeUninit

library/core/src/mem/maybe_uninit.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

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

This is in core and Vec is in alloc. So this link will not resolve: #74481.

Suggested change
/// [`Vec<T>`]: std::vec::Vec<T>
/// [`Vec<T>`]: ../../std/vec/struct.Vec.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks...I was misunderstanding. I will fix it and squash the commit

@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2020
@chansuke chansuke force-pushed the fix-intra-doc-links branch from 8276d39 to 288e9ba Compare August 7, 2020 15:02
jyn514
jyn514 reviewed Aug 7, 2020
View reviewed changes
library/core/src/mem/manually_drop.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// [pinned]: ../pin/index.html
/// [pinned]: crate::pin

library/core/src/mem/manually_drop.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Is ptr in scope in this module? If so this is fine, but otherwise it needs to be crate::ptr or super::ptr.

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 thought that ptr was in scope since it doesn't give me results in unresolved link error... but anyway I will add crate. Thanks

@chansuke chansuke force-pushed the fix-intra-doc-links branch from 288e9ba to a296df0 Compare August 8, 2020 11:20
@jyn514
Copy link
Member

jyn514 commented Aug 8, 2020 •
edited
Loading

Broken links 
  --> library/core/src/mem/manually_drop.rs:59:25
   |
59 | /// [`MaybeUninit<T>`]: MaybeUninit
   |                         ^^^^^^^^^^^ cannot be resolved, ignoring
   |
   = note: `-D intra-doc-link-resolution-failure` implied by `-D warnings`
   = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

error: `[mem::zeroed]` cannot be resolved, ignoring it.
  --> library/core/src/mem/manually_drop.rs:10:11
   |
10 | /// with [`mem::zeroed`] is undefined behavior.
   |           ^^^^^^^^^^^^^ cannot be resolved, ignoring
   |
   = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`

error: `[MaybeUninit]` cannot be resolved, ignoring it.
  --> library/core/src/mem/manually_drop.rs:59:25
   |
59 | /// [`MaybeUninit<T>`]: MaybeUninit
   |                         ^^^^^^^^^^^ cannot be resolved, ignoring
   |
   = help: to escape `[` and `]` characters, just add '\' before them like `\[` or `\]`
``

</details>

Let me know if you have trouble fixing them.

@chansuke chansuke force-pushed the fix-intra-doc-links branch 2 times, most recently from 072a905 to d8536c3 Compare August 14, 2020 14:28
jyn514
jyn514 reviewed Aug 14, 2020
View reviewed changes
library/core/src/mem/manually_drop.rs Outdated
Comment on lines 119 to 120
Copy link
Member

Choose a reason for hiding this comment

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

I don't see anything using drop or into_inner so you can remove these reference links :)

Suggested change
/// [`drop`]: ManuallyDrop::drop
/// [`into_inner`]: ManuallyDrop::into_inner

library/core/src/mem/manually_drop.rs Outdated
Comment on lines 151 to 152
Copy link
Member

Choose a reason for hiding this comment

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

Same here, the links don't seem to be used.

Suggested change
/// [`into_inner`]: ManuallyDrop::into_inner
/// [`drop_in_place`]: crate::ptr::drop_in_place

library/core/src/mem/manually_drop.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this didn't mean to link to crate::mem::zeroed?

Suggested change
/// [`mem::zeroed`]: crate::mem::MaybeUninit::zeroed
/// [`mem::zeroed`]: crate::mem::zeroed

@chansuke chansuke force-pushed the fix-intra-doc-links branch from d8536c3 to 61866bc Compare August 14, 2020 17:37
@jyn514
Copy link
Member

jyn514 commented Aug 14, 2020

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 14, 2020

📌 Commit 61866bc has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 14, 2020
@tmandry tmandry mentioned this pull request Aug 14, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2020
@bors
Auto merge of rust-lang#75538 - tmandry:rollup-i8miv8q, r=tmandry
95879ad 
Rollup of 17 pull requests

Successful merges:

 - rust-lang#73943 (Document the unsafe keyword)
 - rust-lang#74062 (deny(unsafe_op_in_unsafe_fn) in libstd/ffi/c_str.rs)
 - rust-lang#74185 (Remove liballoc unneeded explicit link)
 - rust-lang#74192 (Improve documentation on process::Child.std* fields)
 - rust-lang#74409 (Change Debug impl of SocketAddr and IpAddr to match their Display output)
 - rust-lang#75195 (BTreeMap: purge innocent use of into_kv_mut)
 - rust-lang#75214 (Use intra-doc links in `mem::manually_drop` & `mem::maybe_uninit`)
 - rust-lang#75432 (Switch to intra-doc links in `std::process`)
 - rust-lang#75482 (Clean up E0752 explanation)
 - rust-lang#75501 (Move to intra doc links in std::ffi)
 - rust-lang#75509 (Tweak suggestion for `this` -> `self`)
 - rust-lang#75511 (Do not emit E0228 when it is implied by E0106)
 - rust-lang#75515 (Bump std's libc version to 0.2.74)
 - rust-lang#75517 (Promotion and const interning comments)
 - rust-lang#75519 (BTreeMap: refactor splitpoint and move testing over to unit test)
 - rust-lang#75530 (Switch to intra-doc links in os/raw/*.md)
 - rust-lang#75531 (Migrate unit tests of btree collections to their native breeding ground)

Failed merges:

r? @ghost
@bors bors merged commit ad8c5e1 into rust-lang:master Aug 15, 2020
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jyn514 jyn514 jyn514 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jyn514 jyn514

Labels

A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.47.0

Development

Successfully merging this pull request may close these issues.

6 participants

@chansuke @rust-highfive @jyn514 @bors @cuviper @cramertj