rustdoc: heavily simplify the synthesis of auto trait impls #123340

fmease · 2024-04-01T22:00:21Z

gd --numstat HEAD~2 HEAD src/librustdoc/clean/auto_trait.rs
+315 -705 🟩🟥🟥🟥⬛

As outlined in issue #113015, there are currently 3¹ 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

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

rustbot · 2024-04-01T22:00:31Z

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

fmease · 2024-04-01T22:04:08Z

auto_trait is relatively hot & rustdoc::clean::simplify is not perf optimized:

@bors try @rust-timer queue

fmease · 2024-04-01T22:05:02Z

tests/rustdoc/synthetic_auto/bounds.rs

+
+impl<T> std::marker::Unpin for Inner<T>
+where
+    T: for<'any> Trait<A = (), B<'any> = (), X = ()>,


On master, this gets rendered as:

T: Trait<A = ()> + Trait<B<'any> = ()> + SuperTrait<X = ()>,

bors · 2024-04-01T22:05:20Z

⌛ Trying commit 785787f with merge ce8cfc5...

…mpl-synth, r=<try> rustdoc: heavily simplify the synthesis of auto trait impls `gd --numstat HEAD~2 HEAD src/librustdoc/clean/auto_trait.rs` **+315 -705** 🟩🟥🟥🟥⬛ --- As outlined in issue rust-lang#113015, there are currently 3[^1] 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 mentioned, `simplify` is also flawed but still it's 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” in follow-up PRs in order to finally [fix] rust-lang#113015. Apart from that, I've eliminated all potential sources of *unstable rendering output*. See also rust-lang#119597. I'm pretty sure this fixes rust-lang#119597. This PR does not attempt to fix [any other issues related to synthetic auto trait impls](https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+label%3AA-synthetic-impls%20label%3AA-auto-traits). 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 / more predictable iteration order * Or as a perf opt, `UnordSet` (a thin wrapper around `FxHashSet`) in cases where we never iterate over the set. * 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 rust-lang#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 and therefore * made `auto_traits` less modular 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 (rust-lang#49711). Sorry for not having split this into more commits. If you'd like me to I can try to retroactively separate it into more atomic commits. However, I don't know if that would actually make reviewing this PR easier. r? `@GuillaumeGomez` [^1]: Or even 4 depending on the way you're counting.

src/librustdoc/clean/mod.rs

src/librustdoc/clean/auto_trait.rs

src/librustdoc/clean/utils.rs

fmease · 2024-04-01T22:26:15Z

src/librustdoc/clean/utils.rs

@@ -494,7 +485,7 @@ pub(crate) fn get_auto_trait_and_blanket_impls(
        .sess()
        .prof
        .generic_activity("get_auto_trait_impls")
-        .run(|| AutoTraitFinder::new(cx).get_auto_trait_impls(item_def_id));
+        .run(|| synthesize_auto_trait_impls(cx, item_def_id));


Indeed, the new name synthesize_auto_trait_impls is inconsistent with the established terminology “get […] impls” like get_auto_trait_and_blanket_impls, get_blanket_impls etc. I plan on updating the old terminology in follow-up PRs. “Get” is super misleading and nondescript. We are in fact “computing” those impls. I chose the new term “synthesize” because we call them “synthetic impls” in other places, too. However, I could also go for “generate” if you'd like me to.

fmease · 2024-04-01T23:28:48Z

src/librustdoc/clean/mod.rs

@@ -502,14 +498,17 @@ fn projection_to_path_segment<'tcx>(

 fn clean_generic_param_def<'tcx>(
    def: &ty::GenericParamDef,
+    defaults: ParamDefaults,


I admit this could be perceived as introducing a weird non-modular coupling between clean and clean::auto_trait. Previously, auto_trait would just clean_ty_generics (which calls this function) and then remove the generic parameter defaults afterwards (in a for-loop). This felt a bit unclean.

Adding this param allows us to get rid of the for-loop in auto_trait. Furthermore, it avoids executing the rustc queries type_of & const_param_default and the functions clean_middle_ty & to_string (only to throw away the results) in auto_trait which boosts perf in theory.

bors · 2024-04-01T23:35:39Z

☀️ Try build successful - checks-actions
Build commit: ce8cfc5 (ce8cfc56a2dad6608d5b6fcc472a67f93a427207)

rust-timer · 2024-04-02T04:02:20Z

Finished benchmarking commit (ce8cfc5): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.4%	[-1.4%, -1.4%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 666.866s -> 666.631s (-0.04%)
Artifact size: 315.67 MiB -> 315.57 MiB (-0.03%)

GuillaumeGomez · 2024-04-02T09:23:06Z

This is awesome! Changes look good to me currently so r=me if the PR is ready.

fmease · 2024-04-02T10:33:02Z

Yes, it's ready.
@bors r=GuillaumeGomez

bors · 2024-04-02T10:33:05Z

📌 Commit 069e7f2 has been approved by GuillaumeGomez

It is now in the queue for this repository.

fmease · 2024-04-02T10:33:50Z

@bors rollup-

bors · 2024-04-02T14:14:46Z

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 5dbaafd to master...

rust-timer · 2024-04-02T16:29:41Z

Finished benchmarking commit (5dbaafd): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.4%	[3.4%, 3.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.9%	[3.9%, 3.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.8%	[1.8%, 1.8%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.8%	[1.8%, 1.8%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 667.665s -> 667.636s (-0.00%)
Artifact size: 315.66 MiB -> 315.59 MiB (-0.02%)

fmease · 2024-04-02T16:59:07Z

spurious, not a doc profile

…uillaumeGomez rustdoc: synthetic auto trait impls: accept unresolved region vars for now rust-lang#123348 (comment): > Right, [in rust-lang#123340] I've intentionally changed a `vid_map.get(vid).unwrap_or(r)` to a `vid_map[vid]` making rustdoc panic if `rustc::AutoTraitFinder` returns a region inference variable that cannot be resolved because that is really fishy. I can change it back with a `FIXME: investigate` […]. [O]nce I [fully] understand [the arcane] `rustc::AutoTraitFinder` [I] can fix the underlying issue if there's one. > > `rustc::AutoTraitFinder` can also return placeholder regions `RePlaceholder` which doesn't seem right either and which makes rustdoc ICE, too (we have a GitHub issue for that already[, namely rust-lang#120606]). Fixes rust-lang#123370. Fixes rust-lang#112242. r? `@GuillaumeGomez`

…uillaumeGomez rustdoc: synthetic auto trait impls: accept unresolved region vars for now rust-lang#123348 (comment): > Right, [in rust-lang#123340] I've intentionally changed a `vid_map.get(vid).unwrap_or(r)` to a `vid_map[vid]` making rustdoc panic if `rustc::AutoTraitFinder` returns a region inference variable that cannot be resolved because that is really fishy. I can change it back with a `FIXME: investigate` […]. [O]nce I [fully] understand [the arcane] `rustc::AutoTraitFinder` [I] can fix the underlying issue if there's one. > > `rustc::AutoTraitFinder` can also return placeholder regions `RePlaceholder` which doesn't seem right either and which makes rustdoc ICE, too (we have a GitHub issue for that already[, namely rust-lang#120606]). Fixes rust-lang#123370. Fixes rust-lang#112242. r? ``@GuillaumeGomez``

Rollup merge of rust-lang#123375 - fmease:rustdoc-sati-re-hotfix, r=GuillaumeGomez rustdoc: synthetic auto trait impls: accept unresolved region vars for now rust-lang#123348 (comment): > Right, [in rust-lang#123340] I've intentionally changed a `vid_map.get(vid).unwrap_or(r)` to a `vid_map[vid]` making rustdoc panic if `rustc::AutoTraitFinder` returns a region inference variable that cannot be resolved because that is really fishy. I can change it back with a `FIXME: investigate` […]. [O]nce I [fully] understand [the arcane] `rustc::AutoTraitFinder` [I] can fix the underlying issue if there's one. > > `rustc::AutoTraitFinder` can also return placeholder regions `RePlaceholder` which doesn't seem right either and which makes rustdoc ICE, too (we have a GitHub issue for that already[, namely rust-lang#120606]). Fixes rust-lang#123370. Fixes rust-lang#112242. r? ``@GuillaumeGomez``

…, r=GuillaumeGomez rustdoc: heavily simplify the synthesis of auto trait impls `gd --numstat HEAD~2 HEAD src/librustdoc/clean/auto_trait.rs` **+315 -705** 🟩🟥🟥🟥⬛ --- As outlined in issue #113015, there are currently 3[^1] 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`](rust-lang/rust#123340 (comment)). 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](https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+label%3AA-synthetic-impls%20label%3AA-auto-traits). 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 #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 #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` [^1]: Or even 4 depending on the way you're counting.

Auto trait bounds have changed in one of: - rust-lang/rust#123340. - rust-lang/rust#123375.

tamird · 2024-04-03T10:31:43Z

I believe this PR might have changed the bounds on auto trait impls. See aya-rs/aya#920.

fmease · 2024-04-03T10:48:46Z

Thanks for bringing this to my attention I will look into it later. I tried very hard not add extraneous bounds (for now; cc #111101)
but here we are.

Auto trait bounds have changed in one of: - rust-lang/rust#123340. - rust-lang/rust#123375.

…, r=GuillaumeGomez rustdoc: heavily simplify the synthesis of auto trait impls `gd --numstat HEAD~2 HEAD src/librustdoc/clean/auto_trait.rs` **+315 -705** 🟩🟥🟥🟥⬛ --- As outlined in issue #113015, there are currently 3[^1] 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`](rust-lang/rust#123340 (comment)). 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](https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+label%3AA-synthetic-impls%20label%3AA-auto-traits). 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 #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 #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` [^1]: Or even 4 depending on the way you're counting.

…pls-synth, r=camelid rustdoc: slightly clean up the synthesis of blanket impls Small follow-up to rust-lang#123340 as promised in rust-lang#123340 (comment). No functional changes whatsoever. * inline the over-engineered “type namespace” (struct) `BlanketImplFinder` just like I did with `AutoTraitFinder` in rust-lang#123340 * use the new `synthesize_*` terminology over the old nondescript / misleading `get_*` one * inline a `use super::*;` (not super modular, lead to `clean/mod.rs` (!) accumulating cruft) * use `tracing` properly r? GuillaumeGomez or rustdoc

Rollup merge of rust-lang#123647 - fmease:rustdoc-clean-up-blanket-impls-synth, r=camelid rustdoc: slightly clean up the synthesis of blanket impls Small follow-up to rust-lang#123340 as promised in rust-lang#123340 (comment). No functional changes whatsoever. * inline the over-engineered “type namespace” (struct) `BlanketImplFinder` just like I did with `AutoTraitFinder` in rust-lang#123340 * use the new `synthesize_*` terminology over the old nondescript / misleading `get_*` one * inline a `use super::*;` (not super modular, lead to `clean/mod.rs` (!) accumulating cruft) * use `tracing` properly r? GuillaumeGomez or rustdoc

…m-param-env-clauses, r=GuillaumeGomez rustdoc: synthetic auto: filter out clauses from the implementor's ParamEnv ... not just the elaborated clauses. Fixes another regression introduced by me in rust-lang#123340, oops! Fixes rust-lang#123340 (comment), cc `@tamird.` An earlier local iteration of branch `rustdoc-simplify-auto-trait-impl-synth` (PR rust-lang#123340) contained a fix for issue rust-lang#111101 before I decided to limit the scope. I must've introduced this bug when manually reverting that part of the code. r? `@GuillaumeGomez` or rustdoc

…m-param-env-clauses, r=GuillaumeGomez rustdoc: synthetic auto: filter out clauses from the implementor's ParamEnv ... not just the elaborated clauses. Fixes another regression introduced by me in rust-lang#123340, oops! Fixes rust-lang#123340 (comment), cc ``@tamird.`` An earlier local iteration of branch `rustdoc-simplify-auto-trait-impl-synth` (PR rust-lang#123340) contained a fix for issue rust-lang#111101 before I decided to limit the scope. I must've introduced this bug when manually reverting that part of the code. r? ``@GuillaumeGomez`` or rustdoc

Rollup merge of rust-lang#123638 - fmease:rustdoc-synth-auto-yeet-item-param-env-clauses, r=GuillaumeGomez rustdoc: synthetic auto: filter out clauses from the implementor's ParamEnv ... not just the elaborated clauses. Fixes another regression introduced by me in rust-lang#123340, oops! Fixes rust-lang#123340 (comment), cc ``@tamird.`` An earlier local iteration of branch `rustdoc-simplify-auto-trait-impl-synth` (PR rust-lang#123340) contained a fix for issue rust-lang#111101 before I decided to limit the scope. I must've introduced this bug when manually reverting that part of the code. r? ``@GuillaumeGomez`` or rustdoc

…, r=GuillaumeGomez rustdoc: heavily simplify the synthesis of auto trait impls `gd --numstat HEAD~2 HEAD src/librustdoc/clean/auto_trait.rs` **+315 -705** 🟩🟥🟥🟥⬛ --- As outlined in issue #113015, there are currently 3[^1] 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`](rust-lang/rust#123340 (comment)). 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](https://github.com/rust-lang/rust/issues?q=is%3Aissue+is%3Aopen+label%3AA-synthetic-impls%20label%3AA-auto-traits). 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 #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 #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` [^1]: Or even 4 depending on the way you're counting.

rustdoc: synthetic impls: auto traits: Fx{Hash↦Index}{Map,Set}

cbd593e

rustbot assigned GuillaumeGomez Apr 1, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 1, 2024

fmease changed the title ~~rustdoc: heavily simplify synthesis of auto trait impls~~ rustdoc: heavily simplify the synthesis of auto trait impls Apr 1, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 1, 2024

fmease commented Apr 1, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

fmease force-pushed the rustdoc-simplify-auto-trait-impl-synth branch from 785787f to c8070f5 Compare April 1, 2024 23:39

This comment has been minimized.

Sign in to view

rustdoc: heavily simplify synthesis of auto trait impls

069e7f2

fmease force-pushed the rustdoc-simplify-auto-trait-impl-synth branch from c8070f5 to 069e7f2 Compare April 1, 2024 23:50

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 2, 2024

GuillaumeGomez approved these changes Apr 2, 2024

View reviewed changes

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 Apr 2, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 2, 2024

bors merged commit 5dbaafd into rust-lang:master Apr 2, 2024
12 checks passed

rustbot added this to the 1.79.0 milestone Apr 2, 2024

fmease deleted the rustdoc-simplify-auto-trait-impl-synth branch April 2, 2024 15:00

This was referenced Apr 2, 2024

[WIP] rustdoc (base+json): add support for variances #123219

Draft

[blocked on design] [PoC]: rustdoc (html): add support for variances #123220

Draft

matthiaskrgr mentioned this pull request Apr 2, 2024

rustdoc: ICE: IndexMap: key not found #123370

Closed

fmease mentioned this pull request Apr 2, 2024

rustdoc: synthetic auto trait impls: accept unresolved region vars for now #123375

Merged

tamird added a commit to tamird/aya that referenced this pull request Apr 3, 2024

public-api: regenerate

7258be3

Auto trait bounds have changed in one of: - rust-lang/rust#123340. - rust-lang/rust#123375.

tamird mentioned this pull request Apr 3, 2024

public-api: regenerate aya-rs/aya#920

Merged

tamird added a commit to aya-rs/aya that referenced this pull request Apr 3, 2024

public-api: regenerate

af1a9b3

Auto trait bounds have changed in one of: - rust-lang/rust#123340. - rust-lang/rust#123375.

This was referenced Apr 8, 2024

rustdoc: synthetic auto: filter out clauses from the implementor's ParamEnv #123638

Merged

rustdoc: slightly clean up the synthesis of blanket impls #123647

Merged

fmease mentioned this pull request Apr 15, 2024

rustdoc's resugaring of projection predicates to type bindings is flawed when supertraits are involved #113015

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: heavily simplify the synthesis of auto trait impls #123340

rustdoc: heavily simplify the synthesis of auto trait impls #123340

fmease commented Apr 1, 2024 •

edited

rustbot commented Apr 1, 2024

fmease commented Apr 1, 2024

This comment has been minimized.

fmease Apr 1, 2024

bors commented Apr 1, 2024

fmease Apr 1, 2024

fmease Apr 1, 2024 •

edited

bors commented Apr 1, 2024

This comment has been minimized.

This comment has been minimized.

rust-timer commented Apr 2, 2024

GuillaumeGomez commented Apr 2, 2024

fmease commented Apr 2, 2024

bors commented Apr 2, 2024

fmease commented Apr 2, 2024

bors commented Apr 2, 2024

rust-timer commented Apr 2, 2024

fmease commented Apr 2, 2024

tamird commented Apr 3, 2024

fmease commented Apr 3, 2024 •

edited

rustdoc: heavily simplify the synthesis of auto trait impls #123340

rustdoc: heavily simplify the synthesis of auto trait impls #123340

Conversation

fmease commented Apr 1, 2024 • edited

Footnotes

rustbot commented Apr 1, 2024

fmease commented Apr 1, 2024

This comment has been minimized.

Choose a reason for hiding this comment

bors commented Apr 1, 2024

Choose a reason for hiding this comment

fmease Apr 1, 2024 • edited

Choose a reason for hiding this comment

bors commented Apr 1, 2024

This comment has been minimized.

This comment has been minimized.

rust-timer commented Apr 2, 2024

Overall result: no relevant changes - no action needed

GuillaumeGomez commented Apr 2, 2024

fmease commented Apr 2, 2024

bors commented Apr 2, 2024

fmease commented Apr 2, 2024

bors commented Apr 2, 2024

rust-timer commented Apr 2, 2024

Overall result: ❌ regressions - no action needed

fmease commented Apr 2, 2024

tamird commented Apr 3, 2024

fmease commented Apr 3, 2024 • edited

fmease commented Apr 1, 2024 •

edited

fmease Apr 1, 2024 •

edited

fmease commented Apr 3, 2024 •

edited