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

Use intra-doc links for core/src/slice.mod.rs #75932

Merged
merged 5 commits into from
Aug 31, 2020

Conversation

Amjad50
Copy link
Contributor

@Amjad50 Amjad50 commented Aug 26, 2020

partial help in #75080

r? @jyn514

  • most are using primitive types links, which cannot be used with intra links at the moment
  • also std cannot be referenced in any link, std::ptr::NonNull and std::slice could not be referenced

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jyn514 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 26, 2020
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.

There are a lot more links that can still be converted (e.g.

//! [`std::slice`]: ../../std/slice/index.html
), but this is a good partial fix :)

r=me with comments addressed

library/core/src/slice/mod.rs Outdated Show resolved Hide resolved
library/core/src/slice/mod.rs Outdated Show resolved Hide resolved
library/core/src/slice/mod.rs Show resolved Hide resolved
library/core/src/slice/mod.rs Outdated Show resolved Hide resolved
library/core/src/slice/mod.rs Outdated Show resolved Hide resolved
library/core/src/slice/mod.rs Outdated Show resolved Hide resolved
library/core/src/slice/mod.rs Outdated Show resolved Hide resolved
@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

also std cannot be referenced in any link

Use crate for items that are in core and re-exported in std. If there items are in std or alloc only, you can't use intra-doc links for them.

@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 26, 2020
@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 26, 2020

There are a lot more links that can still be converted (e.g.

//! [`std::slice`]: ../../std/slice/index.html

), but this is a good partial fix :)
r=me with comments addressed

Thanks so much for the coverage 👍 .

I'm not sure how can I fix this, I don't think I can use crate::slice right?

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

You can :) This is linking to https://doc.rust-lang.org/std/slice/index.html, which is a re-export of https://doc.rust-lang.org/core/slice/index.html, which is the current module. So crate::slice or self will go to the same place.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

Oh hmm - it looks like that's not quite true, the std page has more info than core. So you can't change that link, sorry.

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 26, 2020

Yah, thats what made me think of keeping it.

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 26, 2020

Even though this is 7k file, but not many changes

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 26, 2020

Ok, can I link

 /// [valid]: ../../std/ptr/index.html#safety

?

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 26, 2020

It appears yes, I can

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

alloc/slice/fn.from_raw_parts.html:18: broken link - alloc/slice/crate::ptr::NonNull::dangling
alloc/slice/fn.from_raw_parts_mut.html:17: broken link - alloc/slice/crate::ptr::NonNull::dangling

This is #65983, fixed by #73101. If you wait a day or two until the beta bump it will be fixed then; if not you'll have to revert the changes.

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 26, 2020

sorry I didn't run the checker for that.

Ok I guess we can wait for it to be fixed.

Thanks again for the help

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

sorry I didn't run the checker for that.

That's ok, that's what CI is for :)

@jyn514
Copy link
Member

jyn514 commented Aug 28, 2020

Beta was bumped today, so this should be good to go after a rebase :)

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 28, 2020

still broken :(

@jyn514
Copy link
Member

jyn514 commented Aug 28, 2020

@Amjad50 you haven't rebased, so it's showing the test result from the previous compiler. Try rebasing over master.

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 28, 2020

I rebased, when I push to here should I force? or just merge with the previous

@jyn514
Copy link
Member

jyn514 commented Aug 28, 2020

You should force-push, yes. git push -f

@jyn514
Copy link
Member

jyn514 commented Aug 28, 2020

If you merge it defeats the point of rebasing: it shows both the old and new commits, and makes it look in the diff as if you have all the hundreds of commits that you rebased over.

- most are using primitive types links, which cannot be used with intra
  links at the moment
- also `std` cannot be referenced in any link
@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 28, 2020

Is the rebase done correctly? It failed

@jyn514
Copy link
Member

jyn514 commented Aug 28, 2020

Your rebase is fine but the bug was still not fixed :( It's a rustdoc bug of some sort, I'll try to look into it this weekend.

If you can reproduce outside the standard library that would be amazing ❤️ but no problem if you're not sure how.

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 28, 2020

Definitely, I really love contributing. Thanks so much for the help.😊

@jyn514
Copy link
Member

jyn514 commented Aug 29, 2020

I don't understand why this is failing :/ the error is being returned directly from rustc_resolve:

$ RUSTDOC_LOG=rustdoc::passes::collect=debug,rustc_resolve=debug xpy test --stage 1 linkchecker library/alloc
: Some(NodeId(101625)), has_generic_args: false }], opt_ns=Some(MacroNS), record_used=true, path_span=library/alloc/src/lib.rs:1:1: 1:1 (#0), crate_lint=No)
2020-08-29T22:46:53.052491Z DEBUG rustc_resolve: resolve_path ident 0 crate#0 Some(NodeId(101624))
2020-08-29T22:46:53.052494Z DEBUG rustc_resolve: resolve_crate_root(crate#0)
2020-08-29T22:46:53.052496Z DEBUG rustc_resolve: resolve_crate_root: not DollarCrate
2020-08-29T22:46:53.052499Z DEBUG rustc_resolve: resolve_crate_root(crate#0): found no mark (ident.span = library/alloc/src/lib.rs:1:1: 1:1 (#0))
2020-08-29T22:46:53.052502Z DEBUG rustc_resolve: resolve_path ident 1 ptr#0 Some(NodeId(101625))
2020-08-29T22:46:53.053654Z DEBUG rustc_resolve: resolve_path(path=[Segment { ident: crate#0, id: Some(NodeId(101626)), has_generic_args: false }, Segment { ident: ptr#0, id
: Some(NodeId(101627)), has_generic_args: false }], opt_ns=Some(TypeNS), record_used=true, path_span=library/alloc/src/lib.rs:1:1: 1:1 (#0), crate_lint=No)
2020-08-29T22:46:53.053666Z DEBUG rustc_resolve: resolve_path ident 0 crate#0 Some(NodeId(101626))
2020-08-29T22:46:53.053669Z DEBUG rustc_resolve: resolve_crate_root(crate#0)
2020-08-29T22:46:53.053671Z DEBUG rustc_resolve: resolve_crate_root: not DollarCrate
2020-08-29T22:46:53.053673Z DEBUG rustc_resolve: resolve_crate_root(crate#0): found no mark (ident.span = library/alloc/src/lib.rs:1:1: 1:1 (#0))
2020-08-29T22:46:53.053677Z DEBUG rustc_resolve: resolve_path ident 1 ptr#0 Some(NodeId(101627))
2020-08-29T22:46:53.054499Z DEBUG rustdoc::passes::collect_intra_doc_links: crate::ptr resolved to Err(()) in namespace TypeNS with parent DefId(1:6606 ~ core[c85e]::slice[0
])
2020-08-29T22:46:53.055390Z  INFO rustdoc::passes::collect_intra_doc_links: ignoring warning from parent crate: unresolved link to `crate::ptr`

`alloc::slice` uses `core::slice` functions, documentation are copied
from there and the links as well without resolution. `crate::ptr...`
cannot be resolved in `alloc::slice`, but `ptr` itself is imported in
both `alloc::slice` and `core::slice`, so we used that instead.
@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 30, 2020

I think I fixed it, the problem in general is not solved. But for this case, we can use ptr instead of crate::ptr as it is available in both alloc::slice and core::slice

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 30, 2020

So from what I understand, use will copy the function/struct into that location together with it's documentation, and then resolve it from there.

Is there a way to change it so it resolves first and then copy the documentation? And would that affect other documentation processes.

@jyn514
Copy link
Member

jyn514 commented Aug 30, 2020

So from what I understand, use will copy the function/struct into that location together with it's documentation, and then resolve it from there.

Is there a way to change it so it resolves first and then copy the documentation? And would that affect other documentation processes.

The fact that it does this is a bug, and I thought I fixed it in #73101. I don't know why it's still happening.

@jyn514
Copy link
Member

jyn514 commented Aug 30, 2020

In fact if you look at the debug logging it shouldn't be resolving in the current scope: it says itself that it's looking at core: parent DefId(1:6606 ~ core[c85e]::slice[0])

@Amjad50
Copy link
Contributor Author

Amjad50 commented Aug 30, 2020

Should this be merged? Or you think it's better to fix the problem first?

@jyn514
Copy link
Member

jyn514 commented Aug 30, 2020

Let's merge this for now, since the link will keep working even if the rustdoc bug is fixed. I'll open a separate issue for the rustdoc bug.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 30, 2020

📌 Commit 300a007 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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 30, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 30, 2020
Use intra-doc links for `core/src/slice.mod.rs`

partial help in rust-lang#75080

r? @jyn514

- most are using primitive types links, which cannot be used with intra links at the moment
- also `std` cannot be referenced in any link, `std::ptr::NonNull` and `std::slice` could not be referenced
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 31, 2020
Use intra-doc links for `core/src/slice.mod.rs`

partial help in rust-lang#75080

r? @jyn514

- most are using primitive types links, which cannot be used with intra links at the moment
- also `std` cannot be referenced in any link, `std::ptr::NonNull` and `std::slice` could not be referenced
@bors
Copy link
Contributor

bors commented Aug 31, 2020

⌛ Testing commit 300a007 with merge 8bfe289...

@bors
Copy link
Contributor

bors commented Aug 31, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: jyn514
Pushing 8bfe289 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 31, 2020
@bors bors merged commit 8bfe289 into rust-lang:master Aug 31, 2020
@cuviper cuviper added this to the 1.48.0 milestone Nov 17, 2023
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-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants