rustdoc: heavily simplify the synthesis of auto trait impls by fmease · Pull Req...

edited

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::ty data 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 mod auto_trait and reuses the routines found in mod simplify. As alluded to, simplify is also flawed but it's still more complete than auto_trait's routines. See also my review comment over at tests/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_trait more contributor-friendly.

---

• Replace FxHash{Map,Set} with FxIndex{Map,Set} to guarantee a stable iteration order
  • Or as a perf opt, UnordSet (a thin wrapper around FxHashSet) in cases where we never iterate over the set.
  • Yes, we do make use of swap_remove but 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 on rustc_infer being deterministic. I hope that holds.
• Utilizing clean::simplify over the custom “bounds cleaning” routines wipes out the last reference to collect_referenced_late_bound_regions in rustdoc (simplify uses bound_vars) which was a source of instability / unpredictability (cc rustdoc: fix & clean up handling of cross-crate higher-ranked parameters #116388)
• Remove the types RegionTarget and RegionDeps from librustdoc. They were duplicates of the identical types found in rustc. Just import them from rustc. For some reason, they were duplicated when splitting auto_trait in two in Refactor auto trait handling in librustdoc to be accessible from librustc. #49711.
• Get rid of the useless “type namespace” AutoTraitFinder in librustdoc
  • The struct only held a DocContext, it was over-engineered
  • Turn the associated functions into free ones
    • Eliminates rightward drift; increases legibility
  • rustc also contains a useless AutoTraitFinder struct but I plan on removing that in a follow-up PR
• Rename a bunch of methods to be way more descriptive
• Eliminate use super::*;
  • Lead to clean/mod.rs accumulating a lot of unnecessary imports
  • Made auto_traits less modular
• Eliminate a custom TypeFolder: We can just use the rustc helper fold_regions which does that for us

I plan on adding extensive documentation to librustdoc's auto_trait in 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's auto_trait modules to a great degree. I'm slowly digging into the dark details of rustc's auto_trait module again and once I have the full picture I will be able to provide proper docs.

---

While this PR does indeed touch rustc's auto_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 of librustdoc after 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_trait on the left of your screen and the patched one on the right, not joking.

r? @GuillaumeGomez

Footnotes

1. Or even 4 depending on the way you're counting. ↩

compiler-errors, lukas-code, and GuillaumeGomez reacted with heart emoji