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

Remove #[doc(hidden)] from macros in std::prelude. #83155

Closed
wants to merge 1 commit into from

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Mar 15, 2021

Seems like this rustdoc bug was fixed.

Fixes #63268.
cc rust-lang/rfcs#3090 (comment)

r? @jyn514

Seems like this rustdoc bug was fixed.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2021
@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

I just noticed that some of the prelude imports are documented as crate:: and some as core::prelude::v1::: https://doc.rust-lang.org/1.50.0/std/prelude/v1/index.html

This just matches how the paths are expressed in code, but it means that the traits that are now imported together with the identically named macros from the core prelude are also documented as core::prelude::v1:: and not crate:: like the other traits.

@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

Maybe we should just do pub use core::prelude::v1::*; in std instead.

@petrochenkov
Copy link
Contributor

Could you mark this PR as closing #63268?

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Maybe we should just do pub use core::prelude::v1::*; in std instead.

That seems fine to me, but I think it's a T-libs decision. r=me on this specific change, I'll leave it up to you if you want to change it to a glob reexport.

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Could you mark this PR as closing #63268?

Done, good catch!

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.................................................................................................... 9300/11557
.................................................................................................... 9400/11557
.....................................................................................i......i....... 9500/11557
.................................................................................................... 9600/11557
...............................iiiiiii..iiiiii.i.................................................... 9700/11557
.................................................................................................... 9900/11557
.................................................................................................... 10000/11557
.................................................................................................... 10100/11557
.................................................................................................... 10200/11557
---
Suite("src/test/assembly") not skipped for "bootstrap::test::Assembly" -- not in ["src/tools/tidy"]
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 29 tests
iiiiiiiiiiiiiiiiiiiiiiiiiiiii

Suite("src/test/incremental") not skipped for "bootstrap::test::Incremental" -- not in ["src/tools/tidy"]
 finished in 0.067 seconds
Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
Suite("src/test/debuginfo") not skipped for "bootstrap::test::Debuginfo" -- not in ["src/tools/tidy"]
Check compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 116 tests
iiiiiiiiii.i.i..i..i....ii...ii....ii..........iiii.........i.....i...i.......ii.i.ii.....iiii.....i 100/116
test result: ok. 78 passed; 0 failed; 38 ignored; 0 measured; 0 filtered out; finished in 2.45s

 finished in 2.518 seconds
Suite("src/test/ui-fulldeps") not skipped for "bootstrap::test::UiFullDeps" -- not in ["src/tools/tidy"]
---
Building stage0 tool linkchecker (x86_64-unknown-linux-gnu)
   Compiling once_cell v1.4.1
   Compiling linkchecker v0.1.0 (/checkout/src/tools/linkchecker)
    Finished release [optimized] target(s) in 2.06s
std/prelude/v1/index.html:7: broken link - core/macros/builtin/attr.bench.html
std/prelude/v1/index.html:7: broken link - core/macros/builtin/attr.global_allocator.html
std/prelude/v1/index.html:7: broken link - core/macros/builtin/attr.test.html
std/prelude/v1/index.html:7: broken link - core/macros/builtin/attr.test_case.html
std/prelude/v1/index.html:7: broken link - core/macros/builtin/derive.RustcDecodable.html
std/prelude/v1/index.html:7: broken link - core/macros/builtin/derive.RustcEncodable.html
thread 'main' panicked at 'found some broken links', src/tools/linkchecker/main.rs:97:9


command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/linkchecker" "/checkout/obj/build/x86_64-unknown-linux-gnu/doc"
expected success, got: exit code: 101

@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

Looks like it doesn't work yet for the few macros that are not available anywhere in std's public interface (other than the prelude).

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Looks like it doesn't work yet for the few macros that are not available anywhere in std's public interface (other than the prelude).

Could you change it so only those macros are marked as doc(hidden) in the meantime?

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Also, if you mark those as doc(inline), does it show the documentation it should?

@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

I just noticed that some of the prelude imports are documented as crate:: and some as core::prelude::v1::: https://doc.rust-lang.org/1.50.0/std/prelude/v1/index.html

This just matches how the paths are expressed in code, but it means that the traits that are now imported together with the identically named macros from the core prelude are also documented as core::prelude::v1:: and not crate:: like the other traits.

Trying out some stuff.. It's quite tricky/impossible to express them all in terms of crate:: in std, because we want the Debug macro, but not the Debug trait, but they're in the same path in std. We also want std::env the macro, but not std::env the module, etc.

@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

Could you change it so only those macros are marked as doc(hidden) in the meantime?

That's exactly the state before this PR.

Looks like the change for the ones that do work already happened in aa863ca (which seems to have the wrong commit message), as part of #77862

@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

Also, if you mark those as doc(inline), does it show the documentation it should?

Yes, that works fine. (It does break the link to std::alloc::GlobalAlloc in the inlined global_allocator's documentation though, as that's just a hardcoded relative url from core to std.)

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Also, if you mark those as doc(inline), does it show the documentation it should?

Yes, that works fine. (It does break the link to std::alloc::GlobalAlloc in the inlined global_allocator's documentation though, as that's just a hardcoded relative url from core to std.)

That should be fixed by using ../../std (instead of ../std).

@jyn514 jyn514 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. 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 Mar 15, 2021
@m-ou-se
Copy link
Member Author

m-ou-se commented Mar 15, 2021

Closing this, as the issue with PartialOrd, etc. was already solved, and doc(inline)'ing the remaining ones is probably not a great solution. Might open a PR at some point to change the entire std prelude to just glob-import alloc and core's preludes.

@m-ou-se m-ou-se closed this Mar 15, 2021
@m-ou-se m-ou-se deleted the prelude-no-hidden branch March 15, 2021 19:22
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 S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc: Built-in macros are not documented in all the necessary locations
5 participants