rustdoc: heavily simplify the synthesis of auto trait impls#123340
Merged
bors merged 2 commits intorust-lang:masterfrom Apr 2, 2024
Merged
rustdoc: heavily simplify the synthesis of auto trait impls#123340bors merged 2 commits intorust-lang:masterfrom
bors merged 2 commits intorust-lang:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gd --numstat HEAD~2 HEAD src/librustdoc/clean/auto_trait.rs+315 -705 🟩🟥🟥🟥⬛
As outlined in issue #113015, there are currently 31 large separate routines that “clean”
rustc_middle::tydata types related to generics & predicates to rustdoc data types. Every single one has their own kinds of bugs. While I've patched a lot of bugs in each of the routines in the past, it's about time to unify them. This PR is only the first in a series. It completely yanks the custom “bounds cleaning” of modauto_traitand reuses the routines found in modsimplify. As alluded to,simplifyis also flawed but it's still more complete thanauto_trait's routines. See also my review comment over attests/rustdoc/synthetic_auto/bounds.rs.This is preparatory work for rewriting “bounds cleaning” from scratch in follow-up PRs in order to finally [fix] #113015.
Apart from that, I've eliminated all potential sources of instability in the rendered output.
See also #119597. I'm pretty sure this fixes #119597.
This PR does not attempt to fix any other issues related to synthetic auto trait impls.
However, it's definitely meant to be a stepping stone by making
auto_traitmore contributor-friendly.FxHash{Map,Set}withFxIndex{Map,Set}to guarantee a stable iteration orderUnordSet(a thin wrapper aroundFxHashSet) in cases where we never iterate over the set.swap_removebut that shouldn't matter since all the callers are deterministic. It does make the output less “predictable” but it's still better than before. Ofc, I rely onrustc_inferbeing deterministic. I hope that holds.clean::simplifyover the custom “bounds cleaning” routines wipes out the last reference tocollect_referenced_late_bound_regionsin rustdoc (simplifyusesbound_vars) which was a source of instability / unpredictability (cc rustdoc: fix & clean up handling of cross-crate higher-ranked parameters #116388)RegionTargetandRegionDepsfromlibrustdoc. They were duplicates of the identical types found inrustc. Just import them fromrustc. For some reason, they were duplicated when splittingauto_traitin two in Refactor auto trait handling in librustdoc to be accessible from librustc. #49711.AutoTraitFinderinlibrustdocDocContext, it was over-engineeredrustcalso contains a uselessAutoTraitFinderstruct but I plan on removing that in a follow-up PRuse super::*;clean/mod.rsaccumulating a lot of unnecessary importsauto_traitsless modularTypeFolder: We can just use the rustc helperfold_regionswhich does that for usI plan on adding extensive documentation to
librustdoc'sauto_traitin follow-up PRs.I don't want to do that in this PR because further refactoring & bug fix PRs may alter the overall structure of
librustdoc's &rustc'sauto_traitmodules to a great degree. I'm slowly digging into the dark details ofrustc'sauto_traitmodule again and once I have the full picture I will be able to provide proper docs.While this PR does indeed touch
rustc'sauto_trait— mostly tiny refactorings — I argue this PR doesn't need any compiler reviewers next to rustdoc ones since that module falls under the purview of rustdoc — it used to be part oflibrustdocafter all (#49711).Sorry for not having split this into more commits. If you'd like me to I can try to split it into more atomic commits retroactively. However, I don't know if that would actually make reviewing easier. I think the best way to review this might just be to place the master version of
auto_traiton the left of your screen and the patched one on the right, not joking.r? @GuillaumeGomez
Footnotes
Or even 4 depending on the way you're counting. ↩