Move attribute and keyword docs from std to core#158327
Conversation
|
Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
|
Thanks! @bors r+ rollup |
…-keywords-to-core, r=GuillaumeGomez Move attribute and keyword docs from `std` to `core` Move the documentation for attributes and keywords into the `core` crate. Apart from strictly moving the module, I had to make a few small changes to certain docs to avoid using `std` types when possible, as well as fixing a few suggestions related to linking to primitives. Pre-requisite for work on rust-lang#157604. r? @GuillaumeGomez Verified documentation using: `./x doc library/core` and `./x doc library/std`, as well as running doc-tests for both crates.
…-keywords-to-core, r=GuillaumeGomez Move attribute and keyword docs from `std` to `core` Move the documentation for attributes and keywords into the `core` crate. Apart from strictly moving the module, I had to make a few small changes to certain docs to avoid using `std` types when possible, as well as fixing a few suggestions related to linking to primitives. Pre-requisite for work on rust-lang#157604. r? @GuillaumeGomez Verified documentation using: `./x doc library/core` and `./x doc library/std`, as well as running doc-tests for both crates.
|
💔 I suspect this PR failed tests as part of a rollup After fixing the problem, consider running a try job for the failed job before re-approving. Link to failure: #158389 (comment) |
|
This pull request was unapproved. This PR was contained in a rollup (#158389), which was unapproved. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2ce02ed to
8230837
Compare
|
@bors try job=test-various |
This comment has been minimized.
This comment has been minimized.
…o-core, r=<try> Move attribute and keyword docs from `std` to `core` try-job: test-various
|
💔 Test for 6850d53 failed: CI. Failed job:
|
This comment has been minimized.
This comment has been minimized.
|
@jschillem: 🔑 Insufficient privileges: not in try users |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
059494a to
5ebd7f2
Compare
5ebd7f2 to
471fbdf
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Error: Label Ameta can only be set by Rust team members Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
|
Sorry about all the extra labels being added and comments! Borked a rebase 😅 |
| /// | ||
| /// Example of using `become` to implement functional-style `fold`: | ||
| /// ``` | ||
| /// ```ignore (explicit_tail_calls not supported on wasm targets) |
There was a problem hiding this comment.
This looks like a test coverage regression, since it will be ignored on all targets now
There was a problem hiding this comment.
Is there a way to ignore a doc test for a specific target?
When these docs were in std, the test did not run for the wasm32-wasip1 target in CI. However in core it does. The log of the failed try run earlier indicated that the explicit_tail_calls feature is not supported for this target.
There was a problem hiding this comment.
Yes:
#[cfg_attr(target_os = "linux", doc = "```")]
#[cfg_attr(not(target_os = "linux"), doc = "```ignore (not supporting on this target)")]| /// contract that the compiler cannot enforce, documenting it is important. The | ||
| /// standard library has many examples of this, like the following which is an | ||
| /// extract from [`Vec::set_len`]. The `# Safety` section explains the contract | ||
| /// extract from [`ptr::replace`]. The `# Safety` section explains the contract |
There was a problem hiding this comment.
I'd prefer to stick to the original example here -- ptr::replace is a raw pointer method which "feels" quite different from an unsafe method on a typically "safe" type. Also ptr::replace's condition is harder to understand.
There was a problem hiding this comment.
I agree and the goal with changing it was to make the example reference an unsafe method in core instead of std.
I spoke with @GuillaumeGomez about this AlPR. With the move to core, ideally the examples should minimize links to std.
I'll try and find a simpler method in core, and revert if I can't find one.
There was a problem hiding this comment.
I don't think we should artificially limit our discussion of things like the unsafe keyword to examples that can be found in core. Instead, we should write the best docs we can write, using examples from any part of the standard library. unsafe is a language keyword, it's not part of any library, it's placement in core vs std is entirely incidental.
Arguably these docs really should be in the Reference, and then obviously they could use examples from std.
| // Include private modules that exist solely to provide rustdoc | ||
| // documentation for built-in attributes. Using `include!` because rustdoc | ||
| // only looks for these modules at the crate level. | ||
| include!("../../core/src/attribute_docs.rs"); |
There was a problem hiding this comment.
Why are these files included both in libcore and libstd? I guess this is to keep existing links like https://doc.rust-lang.org/nightly/std/keyword.unsafe.html working?
There was a problem hiding this comment.
This pattern was in place for the primitives docs already.
I spoke with some folks during the lang-docs office hour and the assumption is for visibility. Most people will be looking in the std documentation, however it should likely be in both. Additionally yes to avoid breaking existing links.
There was a problem hiding this comment.
When you are in the std docs, you have access to core in any case, so no need to make them appear in both core and std.
There was a problem hiding this comment.
This pattern was in place for the primitives docs already.
Yeah I saw that.
Still, would be good to add a brief comment explaining it.
There was a problem hiding this comment.
Including in both was already set for primitive_docs.rs. I'm assuming this was added for just visibility reasons. But removing attributes and keywords from also being included in std, should I leave primitive_docs.rs included?
There was a problem hiding this comment.
That will mean that looking for fn or any other items will show two items in the search results. Not great.
There was a problem hiding this comment.
Thinking out loud, infra team can likely add redirections for these items.
There was a problem hiding this comment.
That will mean that looking for fn or any other items will show two items in the search results. Not great.
Would it? We don't get duplicates for e.g. bool currently.
There was a problem hiding this comment.
I believe it's only searching in the crate you're in currently. So if if the includes are removed from std, these docs will not be searchable. The user would have to explicitly go to core to find them.
So I don't believe it's show up twice. If anything removing from std would reduce overall visibility as most people are already searching within std.
There was a problem hiding this comment.
Would it? We don't get duplicates for e.g.
boolcurrently.
Ah indeed. That is... surprising. Oh well, if it works let's use it then. =D
|
☔ The latest upstream changes (presumably #158455) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
View all comments
Move the documentation for attributes and keywords into the
corecrate. Apart from strictly moving the module, I had to make a few small changes to certain docs to avoid usingstdtypes when possible, as well as fixing a few suggestions related to linking to primitives.Pre-requisite for work on #157604.
r? @GuillaumeGomez
Verified documentation using:
./x doc library/coreand./x doc library/std, as well as running doc-tests for both crates.