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

rustdoc: Improve calculation of "Impls on Foreign Types" #97613

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jun 1, 2022

The existing code to calculate whether an implementation was on a "Foreign Type" was duplicated across the sidebar generation and the page generation. It also came to the wrong conclusion for some cases where both the trait and the "for" type were re-exports.

This PR extracts the logic into a method of Impl, breaks it into a multi-line method so it can be commented, and adds a case for when the trait and the "for" type came from the same crate. This fixes some cases - like the platform-specific integer types (__m256, __m128, etc). But it doesn't fix all cases. See the screenshots below.

Before:

After:

The remaining types (CString, NulError, etc) are all from the alloc crate, while the Clone trait is from the core crate. Since CString and Clone are both re-exported by std, they are logically local to each other, but I couldn't figure out a good way to detect that in this code. I figure this is still a good step forward.

Related: #97610

r? @camelid

@jsha jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate labels Jun 1, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 1, 2022
@jsha jsha force-pushed the implementation-is-on-local-type branch from e1885d8 to d1cb64f Compare June 1, 2022 06:16
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member

Looks like a good idea to me.

@jsha jsha force-pushed the implementation-is-on-local-type branch from d1cb64f to 37d3638 Compare June 1, 2022 16:15
@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 1, 2022

📌 Commit 37d3638 has been approved by GuillaumeGomez

@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 Jun 1, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 1, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#95594 (Additional `*mut [T]` methods)
 - rust-lang#97130 (rustdoc: avoid including impl blocks with filled-in generics)
 - rust-lang#97166 (Move conditions out of recover/report functions.)
 - rust-lang#97605 (Mention filename in suggestion when it differs from primary span)
 - rust-lang#97613 (rustdoc: Improve calculation of "Impls on Foreign Types")
 - rust-lang#97626 (rename PointerAddress → PointerExposeAddress)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a96e71c into rust-lang:master Jun 2, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 2, 2022
@jsha jsha deleted the implementation-is-on-local-type branch June 2, 2022 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants