Skip to content

rustdoc: Separate the caches for synthetic auto trait & blanket impls#157171

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
fmease:rustdoc-sep-caches-auto-blanket
May 31, 2026
Merged

rustdoc: Separate the caches for synthetic auto trait & blanket impls#157171
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
fmease:rustdoc-sep-caches-auto-blanket

Conversation

@fmease

@fmease fmease commented May 30, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Fixes #148980.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels May 30, 2026
@fmease

fmease commented May 30, 2026

Copy link
Copy Markdown
Member Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 30, 2026
@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request May 30, 2026
@bors
Auto merge of #157171 - fmease:rustdoc-sep-caches-auto-blanket, r=<try>
f14d268 
 rustdoc: Separate the caches for synthetic auto trait & blanket impls
@rust-bors

rust-bors Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f14d268 (f14d2688a5033df0ddc0f711bc16c01326d36654, parent: c58275e0369d09fc3959b8ba87dcbcbe73797465)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer

rust-timer commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f14d268): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

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

Max RSS (memory usage)

Results (secondary -5.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -5.5% [-5.5%, -5.4%] 2
All ❌✅ (primary) - - 0

Cycles

Results (primary 2.4%, secondary 3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Binary size

Results (primary -0.1%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.1% [-0.1%, -0.0%] 19
Improvements ✅
(secondary)		 -0.1% [-0.1%, -0.0%] 18
All ❌✅ (primary) -0.1% [-0.1%, -0.0%] 19

Bootstrap: 515.367s -> 515.314s (-0.01%)
Artifact size: 400.82 MiB -> 400.68 MiB (-0.03%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 30, 2026
fmease
fmease commented May 31, 2026
View reviewed changes
Comment thread src/librustdoc/core.rs
Comment on lines +60 to +63
// FIXME: I'm pretty that the only reason we "need" these caches is because we also invoke
// `synthesize_auto_trait_and_blanket_impls` on all impls(!) for primitive types
// instead of calling it only once per primitive type (see also #97129).
// Get rid of that jank and remove both caches!

@fmease fmease May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have the time or leisure to investigate and implement this now. However, this issue does occupy my mind and I'll tackle it soon(tm).

Tagging the corresp. GH issue: #97129.

View changes since the review

@fmease fmease marked this pull request as ready for review May 31, 2026 07:05
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 31, 2026
@rustbot

rustbot commented May 31, 2026

Copy link
Copy Markdown
Collaborator

r? @lolbinarycat

rustbot has assigned @lolbinarycat.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: rustdoc
  • rustdoc expanded to 9 candidates
  • Random selection from GuillaumeGomez, camelid, lolbinarycat, notriddle

fmease
fmease commented May 31, 2026
View reviewed changes
Comment thread tests/rustdoc-html/impl/blanket-impl-trumps-auto-trait-impl.rs
// 'Marker for Subject'

//@ has - '//*[@id="blanket-implementations-list"]//*[@class="impl"]' \
// 'Marker for Twhere T: Copy'

@fmease fmease May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it has to be Twhere for now, T where would fail the htmldocck check. It seems htmldocck incorrectly strips HTML tags. For reference, the HTML looks like for T<div class="where">where.

IIRC I stumble upon this quite often (unsurprisingly) but I usually split the test expectation into two lines, something like Marker for T\ and where T: Copy which looks better and "hides" this issue (despite being equivalent to what's written here). This bug definitely needs addressing at some point.

View changes since the review

@lolbinarycat lolbinarycat May 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selfishly i would want a whitespace node in many of these places for better graceful degradation on niche browsers without css, but i understand that's not really a priority.

@lolbinarycat

lolbinarycat commented May 31, 2026

Copy link
Copy Markdown
Contributor

@bors r+ rollup

thanks for splitting the commits, it really helped with the review!

@rust-bors

rust-bors Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 7478632 has been approved by lolbinarycat

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 May 31, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request May 31, 2026
@jhpratt
Rollup merge of rust-lang#157171 - fmease:rustdoc-sep-caches-auto-bla…
925f260 
…nket, r=lolbinarycat

 rustdoc: Separate the caches for synthetic auto trait & blanket impls

Fixes rust-lang#148980.
This was referenced May 31, 2026
Closed
Merged
rust-bors Bot pushed a commit that referenced this pull request May 31, 2026
@bors
Auto merge of #157219 - jhpratt:rollup-70ooTMg, r=jhpratt
14210df 
Rollup of 4 pull requests

Successful merges:

 - #157192 (directory handles: add Dir::metadata)
 - #157171 ( rustdoc: Separate the caches for synthetic auto trait & blanket impls)
 - #157185 (Don't lower `inner_span` multiple times)
 - #157211 (use custom `Exhaustive` enum instead of `bool`)
@rust-bors rust-bors Bot merged commit 9fc5f90 into rust-lang:main May 31, 2026
13 checks passed
@rustbot rustbot added this to the 1.98.0 milestone May 31, 2026
rust-timer added a commit that referenced this pull request May 31, 2026
@rust-timer
Unrolled build for #157171
ec14613 
Rollup merge of #157171 - fmease:rustdoc-sep-caches-auto-blanket, r=lolbinarycat

 rustdoc: Separate the caches for synthetic auto trait & blanket impls

Fixes #148980.
@fmease fmease deleted the rustdoc-sep-caches-auto-blanket branch June 1, 2026 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lolbinarycat lolbinarycat lolbinarycat left review comments

Assignees

@lolbinarycat lolbinarycat

Labels

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. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Milestone

1.98.0

Development

Successfully merging this pull request may close these issues.

rustdoc doesn't consider blanket(!) impls of auto(!) traits

4 participants

@fmease @rust-timer @rustbot @lolbinarycat