Fix intra-doc links for inherent impls that are both lang items and not the default impl by jyn514 · Pull Request #75649 · rust-lang/rust

jyn514 · 2020-08-17T22:59:08Z

I found in #75464 (comment) that str::to_uppercase() doesn't resolve while str::trim() does. The only real difference is that to_uppercase is defined in alloc, while trim is defined in core. It turns out that rustdoc was ignoring lang_items.str_alloc_impl() - it saw them in collect_trait_impls, but not for intra-doc links.

This uses the same impls for all parts of rustdoc, so that there can be no more inconsistency. It does have the slight downside that the matches are no longer exhaustive but it will be very clear if a new lang item is missed because it will panic when you try to document it (and if you don't document it, does rustdoc really need to know about it?).

~~This needs a test case (probably just str::to_uppercase).~~ Added.

This is best reviewed commit-by-commit.

r? @GuillaumeGomez

rust-highfive · 2020-08-17T22:59:11Z

Some changes occurred in intra-doc-links.

cc @jyn514

jyn514 · 2020-08-17T23:32:03Z

This does keep one small change from #74489, which is to use find_by_name_and_namespace instead of filter_by_name_unhygenic. But that's a good change anyway and meant fewer rebase conflicts.

GuillaumeGomez · 2020-08-18T15:26:25Z

Nice work!

@bors: r+

bors · 2020-08-18T15:26:27Z

📌 Commit 7be824e has been approved by GuillaumeGomez

@GuillaumeGomez

…aumeGomez Fix intra-doc links for inherent impls that are both lang items and not the default impl I found in rust-lang#75464 (comment) that `str::to_uppercase()` doesn't resolve while `str::trim()` does. The only real difference is that `to_uppercase` is defined in `alloc`, while trim is defined in `core`. It turns out that rustdoc was ignoring `lang_items.str_alloc_impl()` - it saw them in `collect_trait_impls`, but not for intra-doc links. This uses the same `impls` for all parts of rustdoc, so that there can be no more inconsistency. It does have the slight downside that the matches are no longer exhaustive but it will be very clear if a new lang item is missed because it will panic when you try to document it (and if you don't document it, does rustdoc really need to know about it?). ~~This needs a test case (probably just `str::to_uppercase`).~~ Added. This is best reviewed commit-by-commit. r? @GuillaumeGomez

@GuillaumeGomez

…aumeGomez Fix intra-doc links for inherent impls that are both lang items and not the default impl I found in rust-lang#75464 (comment) that `str::to_uppercase()` doesn't resolve while `str::trim()` does. The only real difference is that `to_uppercase` is defined in `alloc`, while trim is defined in `core`. It turns out that rustdoc was ignoring `lang_items.str_alloc_impl()` - it saw them in `collect_trait_impls`, but not for intra-doc links. This uses the same `impls` for all parts of rustdoc, so that there can be no more inconsistency. It does have the slight downside that the matches are no longer exhaustive but it will be very clear if a new lang item is missed because it will panic when you try to document it (and if you don't document it, does rustdoc really need to know about it?). ~~This needs a test case (probably just `str::to_uppercase`).~~ Added. This is best reviewed commit-by-commit. r? @GuillaumeGomez

@GuillaumeGomez

…aumeGomez Fix intra-doc links for inherent impls that are both lang items and not the default impl I found in rust-lang#75464 (comment) that `str::to_uppercase()` doesn't resolve while `str::trim()` does. The only real difference is that `to_uppercase` is defined in `alloc`, while trim is defined in `core`. It turns out that rustdoc was ignoring `lang_items.str_alloc_impl()` - it saw them in `collect_trait_impls`, but not for intra-doc links. This uses the same `impls` for all parts of rustdoc, so that there can be no more inconsistency. It does have the slight downside that the matches are no longer exhaustive but it will be very clear if a new lang item is missed because it will panic when you try to document it (and if you don't document it, does rustdoc really need to know about it?). ~~This needs a test case (probably just `str::to_uppercase`).~~ Added. This is best reviewed commit-by-commit. r? @GuillaumeGomez

tmandry · 2020-08-19T05:12:31Z

Failed in #75688 (comment) due to Cargo.lock being out of date
@bors r-

bors · 2020-08-19T09:32:25Z

☔ The latest upstream changes (presumably #75692) made this pull request unmergeable. Please resolve the merge conflicts.

jyn514 · 2020-08-19T13:00:43Z

@bors r=GuilluameGomez

bors · 2020-08-19T13:00:44Z

📌 Commit 570b0d9 has been approved by GuilluameGomez

jyn514 · 2020-08-19T14:26:42Z

@bors r-

I want to add a test for f32 and f64 methods.

jyn514 · 2020-08-19T15:16:22Z

@bors r=@GuillaumeGomez

Tagging you so you can look at the changes if you want :) but it's only adding a new test.

bors · 2020-08-19T15:16:24Z

📌 Commit aff01f8 has been approved by guillaumegomez

@GuillaumeGomez

…aumegomez Fix intra-doc links for inherent impls that are both lang items and not the default impl I found in rust-lang#75464 (comment) that `str::to_uppercase()` doesn't resolve while `str::trim()` does. The only real difference is that `to_uppercase` is defined in `alloc`, while trim is defined in `core`. It turns out that rustdoc was ignoring `lang_items.str_alloc_impl()` - it saw them in `collect_trait_impls`, but not for intra-doc links. This uses the same `impls` for all parts of rustdoc, so that there can be no more inconsistency. It does have the slight downside that the matches are no longer exhaustive but it will be very clear if a new lang item is missed because it will panic when you try to document it (and if you don't document it, does rustdoc really need to know about it?). ~~This needs a test case (probably just `str::to_uppercase`).~~ Added. This is best reviewed commit-by-commit. r? @GuillaumeGomez

@ghost

Rollup of 7 pull requests Successful merges: - rust-lang#75069 (move const param structural match checks to wfcheck) - rust-lang#75587 (mir building: fix some comments) - rust-lang#75593 (Adjust installation place for compiler docs) - rust-lang#75648 (Make OnceCell<T> transparent to dropck) - rust-lang#75649 (Fix intra-doc links for inherent impls that are both lang items and not the default impl) - rust-lang#75674 (Move to intra doc links for std::io) - rust-lang#75696 (Remove `#[cfg(miri)]` from OnceCell tests) Failed merges: r? @ghost

jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Aug 17, 2020

rust-highfive assigned GuillaumeGomez Aug 17, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2020

This comment has been minimized.
Sign in to view

jyn514 force-pushed the inherent-lang-impls branch from af913c5 to 1b74f04Compare August 17, 2020 23:30

jyn514 changed the title ~~[WIP] Fix intra-doc links for inherent impls that are both lang items and not the default impl~~Fix intra-doc links for inherent impls that are both lang items and not the default implAug 17, 2020

jyn514 mentioned this pull request Aug 17, 2020
Move to intra doc links for ascii.rs and panic.rs #75464
Merged

jyn514 force-pushed the inherent-lang-impls branch from 3aae334 to 7be824eCompare August 18, 2020 00:19

GuillaumeGomez reviewed Aug 18, 2020
View reviewed changes

Comment threadsrc/librustdoc/clean/types.rs Outdated

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 18, 2020

tmandry mentioned this pull request Aug 18, 2020
Rollup of 10 pull requests #75683
Closed

tmandry mentioned this pull request Aug 18, 2020
Rollup of 10 pull requests #75684
Closed

tmandry mentioned this pull request Aug 19, 2020
Rollup of 11 pull requests #75688
Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 19, 2020

jyn514 added 5 commits August 19, 2020 08:18

impl_for_type -> PrimitiveType::impls
06d6d3d

Allow reusing the code in collect_trait_impls
9cf2fa8

Use impls for intra doc links as well
219e93d

xpy fmt
121974e

Add a test
570b0d9

jyn514 force-pushed the inherent-lang-impls branch from 7be824e to 570b0d9Compare August 19, 2020 12:26

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 19, 2020

jyn514 mentioned this pull request Aug 19, 2020
Move to intra-doc links for /library/core/src/intrinsics.rs #75705
Merged

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 19, 2020

Add test for f32 and f64 methods
aff01f8

jyn514 force-pushed the inherent-lang-impls branch from 2fa9719 to aff01f8Compare August 19, 2020 14:54

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 19, 2020

tmandry mentioned this pull request Aug 19, 2020
Rollup of 7 pull requests #75712
Closed

tmandry mentioned this pull request Aug 19, 2020
Rollup of 7 pull requests #75715
Merged

bors merged commit 7d14077 into rust-lang:masterAug 19, 2020

jyn514 deleted the inherent-lang-impls branch August 19, 2020 20:34

This was referenced Aug 23, 2020
Stabilize intra-doc links #74430
Merged
Switch to intra-doc links in core/src/{convert,iter}/mod.rs #75969
Merged

cuviper added this to the 1.47.0 milestone May 2, 2024

Navigation Menu

Uh oh!

Uh oh!

Fix intra-doc links for inherent impls that are both lang items and not the default impl - #75649

Fix intra-doc links for inherent impls that are both lang items and not the default impl#75649
bors merged 9 commits into
rust-lang:masterfrom
jyn514:inherent-lang-impls

jyn514 commented Aug 17, 2020•
edited
Loading

Uh oh!

rust-highfive commented Aug 17, 2020

Uh oh!

This comment has been minimized.

jyn514 commented Aug 17, 2020

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 18, 2020

Uh oh!

bors commented Aug 18, 2020

Uh oh!

tmandry commented Aug 19, 2020

Uh oh!

bors commented Aug 19, 2020

Uh oh!

jyn514 commented Aug 19, 2020

Uh oh!

bors commented Aug 19, 2020

Uh oh!

jyn514 commented Aug 19, 2020

Uh oh!

jyn514 commented Aug 19, 2020

Uh oh!

bors commented Aug 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Uh oh!

Conversation

jyn514 commented Aug 17, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Aug 17, 2020

Uh oh!

This comment has been minimized.

jyn514 commented Aug 17, 2020

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 18, 2020

Uh oh!

bors commented Aug 18, 2020

Uh oh!

tmandry commented Aug 19, 2020

Uh oh!

bors commented Aug 19, 2020

Uh oh!

jyn514 commented Aug 19, 2020

Uh oh!

bors commented Aug 19, 2020

Uh oh!

jyn514 commented Aug 19, 2020

Uh oh!

jyn514 commented Aug 19, 2020

Uh oh!

bors commented Aug 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jyn514 commented Aug 17, 2020•
edited
Loading