rustdoc: Correctness & perf improvements to link-to-definition by fmease · Pull Request #156413 · rust-lang/rust

fmease · 2026-05-10T18:50:29Z

Rewrite the way we resolve type-dependent paths (incl. method calls) in SpanMapVisitor. Instead of trying to (re)find the enclosing body owner each time we encounter such a node, potentially calling typeck_body several times per body, keep track of the "active" BodyId in the visitor and cache the corresponding TypeckResults. The "active" BodyId is updated in visit_nested_body (add) and in visit_item (remove).

This fixes a class of ICEs where we tried to look up the definition of type-relative paths located in non-body items nested in bodies, in the overarching body which is never correct. rustdoc@main has a hack to fix this issue for paths specifically found in assoc types in impls.

Fixes #147882. Fixes #147057. Fixes #155327. Fixes #149089. Fixes #150153. Fixes #156418.
Supersedes #147911.

--generate-link-to-definition is not enabled by default, so we can't perf this as is. Instead, I'm benching them against PR #156348 in PR #156355 using the same parent commit for the try-builds. For context, LTD is extremely costly as the perf run for the baseline PR shows: #156348 (comment).

"Absolute results": #156355 (comment).
"Relative results": https://perf.rust-lang.org/compare.html?....

Excerpt of the relative results:

Primary:

Benchmark	Profile	Scenario	Backend	Target	% Change	Significance Threshold	Significance Factor
nalgebra-0.33.0	doc	full	llvm	x64	-5.82%	0.20%	29.09x
diesel-2.2.10	doc	full	llvm	x64	-3.41%	0.20%	17.05x
image-0.25.6	doc	full	llvm	x64	-2.03%	0.20%	10.16x
regex-automata-0.4.8	doc	full	llvm	x64	-1.94%	0.20%	9.71x
cargo-0.87.1	doc	full	llvm	x64	-1.64%	0.20%	8.21x
clap_derive-4.5.32	doc	full	llvm	x64	-1.63%	0.20%	8.14x
cranelift-codegen-0.119.0	doc	full	llvm	x64	-1.48%	0.20%	7.41x
syn-2.0.101	doc	full	llvm	x64	-1.07%	0.20%	5.35x
ripgrep-14.1.1	doc	full	llvm	x64	-0.96%	0.20%	4.78x
stm32f4-0.15.1	doc	full	llvm	x64	-0.80%	0.20%	4.01x
serde-1.0.219	doc	full	llvm	x64	-0.67%	0.20%	3.33x
hyper-1.6.0	doc	full	llvm	x64	-0.66%	0.20%	3.30x
eza-0.21.2	doc	full	llvm	x64	-0.55%	0.20%	2.74x
unicode-normalization-0.1.24	doc	full	llvm	x64	-0.49%	0.20%	2.45x
serde_derive-1.0.219	doc	full	llvm	x64	-0.45%	0.20%	2.26x
typenum-1.18.0	doc	full	llvm	x64	-0.42%	0.20%	2.08x
bitmaps-3.2.1	doc	full	llvm	x64	-0.32%	0.20%	1.61x

Secondary:

Benchmark	Profile	Scenario	Backend	Target	% Change	Significance Threshold	Significance Factor
deep-vector	doc	full	llvm	x64	-23.96%	0.20%	119.78x
large-workspace	doc	full	llvm	x64	-2.16%	0.20%	10.81x
deeply-nested-multi	doc	full	llvm	x64	-1.35%	0.20%	6.75x
serde-1.0.219-threads4	doc	full	llvm	x64	-0.67%	0.20%	3.33x
wg-grammar	doc	full	llvm	x64	-0.32%	0.20%	1.62x
tt-muncher	opt	full	llvm	x64	-0.31%	0.20%	1.56x

nalgebra-0.33.0:

Query/Function	Time (%)	Time (s)	Time delta	Executions	Executions delta	Hits	Hits delta
Totals	109.57%	2.185	-0.138 (-5.9%)	1614146	-5602 (-0.3%)	16983840	-1436455 (-7.8%)
typeck_root	15.75%	0.344	-0.105 (-23.4%)	1704	-954 (-35.9%)	393	-6758 (-94.5%)
...	...	...	...	...	...	...	...

GuillaumeGomez · 2026-05-10T20:55:58Z

Thanks! Please ping me once ready for review. :)

fmease · 2026-05-12T20:19:33Z

@@ -653,6 +653,15 @@ add the `--scrape-tests` flag.
 This flag enables the generation of links in the source code pages which allow the reader
 to jump to a type definition.



Just documenting pre-existing behavior.

View changes since the review

fmease · 2026-05-12T20:24:46Z

-    type X;
-}
-
-pub struct F<T: C>(pub T::X);


This test file is no longer necessary. You added this regression test for an ICE I reported during review (#135771 (comment)) but now assoc-items.rs tests this case among myriads of others (see "tag" Item, AssocTy, TypeRelative).

View changes since the review

fmease · 2026-05-12T20:25:35Z

superseded by tests/rustdoc-ui/generate-link-to-definition/items-nested-in-bodies.rs

View changes since the review

fmease · 2026-05-12T20:26:24Z

@GuillaumeGomez, ready for review. Best reviewed commit by commit. See the PR description for details about the perf improvements.

fmease · 2026-05-12T20:47:19Z

This is a really horrible diff, I'm sorry. I fully replaced this file, better look at the source directly.

View changes since the review

fmease · 2026-05-12T20:48:07Z

-// These two links must not change and in particular must contain `/derive.`!
-//@ has - '//a[@href="{{channel}}/core/fmt/macros/derive.Debug.html"]' 'Debug'
-//@ has - '//a[@href="{{channel}}/core/cmp/derive.PartialEq.html"]' 'PartialEq'


This already gets tested here:

rust/tests/rustdoc-html/jump-to-def/derive-macro.rs

Lines 21 to 22 in 2aabf3c

//@ has - '//a[@href="{{channel}}/core/fmt/macros/derive.Debug.html"]' 'Debug'

//@ has - '//a[@href="{{channel}}/core/cmp/derive.PartialEq.html"]' 'PartialEq'

View changes since the review

GuillaumeGomez · 2026-05-12T21:37:11Z

This is a much smaller change than I expected. Looks all good to me, thanks! Feel free to r+ if the PR is ready. =D

fmease · 2026-05-12T21:42:22Z

It's ready :) @bors r=GuillaumeGomez rollup (unstable, gated behind flag, perf change not observable)

rust-bors · 2026-05-12T21:42:26Z

📌 Commit 79a5881 has been approved by GuillaumeGomez

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

…laumeGomez rustdoc: Correctness & perf improvements to link-to-definition Rewrite the way we resolve type-dependent paths (incl. method calls) in `SpanMapVisitor`. Instead of trying to (re)find the enclosing body owner each time we encounter such a node, potentially calling `typeck_body` several times per body, keep track of the "active" `BodyId` in the visitor and cache the corresponding `TypeckResults`. The "active" `BodyId` is updated in `visit_nested_body` (add) and in `visit_item` (remove). This fixes a class of ICEs where we tried to look up the definition of type-relative paths located in non-body items nested in bodies, in the overarching body which is never correct. rustdoc@main has a hack to fix this issue for paths specifically found in assoc types in impls. Fixes rust-lang#147882. Fixes rust-lang#147057. Fixes rust-lang#155327. Fixes rust-lang#149089. Fixes rust-lang#150153. Supersedes rust-lang#147911. --- `--generate-link-to-definition` is not enabled by default, so we can't perf this as is. Instead, I'm benching them against PR rust-lang#156348 in PR rust-lang#156355 using the same parent commit for the try-builds. For context, LTD is *extremely* costly as the perf run for the baseline PR shows: rust-lang#156348 (comment). "Absolute results": rust-lang#156355 (comment). "Relative results": [https://perf.rust-lang.org/compare.html?...](https://perf.rust-lang.org/compare.html?start=68c2bff6cb08a87e59246064a6a9f37098e22c3f&end=78d15de5525370011388c8f63847e873c4de14ed&stat=instructions%3Au). Excerpt of the relative results: Primary: Benchmark | Profile | Scenario | Backend | Target | % Change | Significance Threshold | Significance Factor ---|---|---|---|---|---|---|--- nalgebra-0.33.0 | doc | full | llvm | x64 | -5.82% | 0.20% | 29.09x diesel-2.2.10 | doc | full | llvm | x64 | -3.41% | 0.20% | 17.05x image-0.25.6 | doc | full | llvm | x64 | -2.03% | 0.20% | 10.16x regex-automata-0.4.8 | doc | full | llvm | x64 | -1.94% | 0.20% | 9.71x cargo-0.87.1 | doc | full | llvm | x64 | -1.64% | 0.20% | 8.21x clap_derive-4.5.32 | doc | full | llvm | x64 | -1.63% | 0.20% | 8.14x cranelift-codegen-0.119.0 | doc | full | llvm | x64 | -1.48% | 0.20% | 7.41x syn-2.0.101 | doc | full | llvm | x64 | -1.07% | 0.20% | 5.35x ripgrep-14.1.1 | doc | full | llvm | x64 | -0.96% | 0.20% | 4.78x stm32f4-0.15.1 | doc | full | llvm | x64 | -0.80% | 0.20% | 4.01x serde-1.0.219 | doc | full | llvm | x64 | -0.67% | 0.20% | 3.33x hyper-1.6.0 | doc | full | llvm | x64 | -0.66% | 0.20% | 3.30x eza-0.21.2 | doc | full | llvm | x64 | -0.55% | 0.20% | 2.74x unicode-normalization-0.1.24 | doc | full | llvm | x64 | -0.49% | 0.20% | 2.45x serde_derive-1.0.219 | doc | full | llvm | x64 | -0.45% | 0.20% | 2.26x typenum-1.18.0 | doc | full | llvm | x64 | -0.42% | 0.20% | 2.08x bitmaps-3.2.1 | doc | full | llvm | x64 | -0.32% | 0.20% | 1.61x Secondary: Benchmark | Profile | Scenario | Backend | Target | % Change | Significance Threshold | Significance Factor ---|---|---|---|---|---|---|--- deep-vector | doc | full | llvm | x64 | -23.96% | 0.20% | 119.78x large-workspace | doc | full | llvm | x64 | -2.16% | 0.20% | 10.81x deeply-nested-multi | doc | full | llvm | x64 | -1.35% | 0.20% | 6.75x serde-1.0.219-threads4 | doc | full | llvm | x64 | -0.67% | 0.20% | 3.33x wg-grammar | doc | full | llvm | x64 | -0.32% | 0.20% | 1.62x tt-muncher | opt | full | llvm | x64 | -0.31% | 0.20% | 1.56x nalgebra-0.33.0: Query/Function | Time (%) | Time (s) | Time delta | Executions | Executions delta | Hits | Hits delta ---|---|---|---|---|---|---|--- Totals | 109.57% | 2.185 | -0.138 (-5.9%) | 1614146 | -5602 (-0.3%) | 16983840 | -1436455 (-7.8%) typeck_root | 15.75% | 0.344 | -0.105 (-23.4%) | 1704 | -954 (-35.9%) | 393 | -6758 (-94.5%) ...|...|...|...|...|...|...|...

…uwer Rollup of 3 pull requests Successful merges: - #156504 (bootstrap: remap windows-style OUT_DIR paths) - #156325 (Don't treat const param default as projection) - #156413 (rustdoc: Correctness & perf improvements to link-to-definition)

fmease · 2026-05-13T00:42:33Z

Apparently I need to fix #156418 here, so we can document std again; it's more trigger-happy now that we call typeck fewer times.

fmease · 2026-05-13T00:46:02Z

@bors r-

rust-bors · 2026-05-13T00:46:09Z

This pull request was unapproved.

This PR was contained in a rollup (#156514), which was unapproved.

View changes since this unapproval

…e it always is `rustc_resolve` doesn't resolve type-relative paths since that's the job of HIR ty lowering and HIR typeck. `segment.res` comes from `rustc_resolve` and is thus always `Res::Err`. So just try to obtain the `TypeckResults` immediately since they contain the actual resolution as deduced by HIR typeck.

Don't unnecessarily try to obtain the type-dependent definition of callees in `visit_expr`, just let `visit_qpath` handle callees. This means that for callees that are * `Resolved` paths (the majority of callees) we don't try to `typeck` the enclosing body which should improve perf if the body doesn't contain any type-dependent definitions. * actually `TypeRelative` paths we don't resolve them twice (with slightly different spans)

rustbot · 2026-05-13T01:51:59Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

rust-bors · 2026-05-13T01:51:59Z

⚠️ A new commit 96b43c65c708566e679761ec3a874cef6d50c3b0 was pushed.

This PR was contained in a rollup (#156514), which was closed.

fmease force-pushed the rustdoc-ltd-improvs branch from 710fac1 to 359b42c Compare May 10, 2026 19:02

fmease commented May 10, 2026

View reviewed changes

Comment thread src/librustdoc/html/render/span_map.rs Outdated

fmease assigned GuillaumeGomez May 10, 2026

fmease mentioned this pull request May 10, 2026

rustdoc ICEs on free calls, method calls & type-relative paths in anon consts under --generate-link-to-definition #156418

Open

fmease force-pushed the rustdoc-ltd-improvs branch from 359b42c to 53d119e Compare May 11, 2026 12:09

This comment has been minimized.

Sign in to view

fmease force-pushed the rustdoc-ltd-improvs branch 4 times, most recently from 73d1fc8 to 79a5881 Compare May 12, 2026 20:05

fmease changed the title ~~[WIP] rustdoc: Correctness & perf improvements to link-to-definition~~ rustdoc: Correctness & perf improvements to link-to-definition May 12, 2026

fmease marked this pull request as ready for review May 12, 2026 20:18

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 12, 2026

fmease commented May 12, 2026

View reviewed changes

GuillaumeGomez approved these changes May 12, 2026

View reviewed changes

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 12, 2026

JonathanBrouwer mentioned this pull request May 12, 2026

Rollup of 3 pull requests #156514

Closed

rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 13, 2026

fmease added 5 commits May 13, 2026 03:51

rustdoc: SpanMapVisitor: Cache TypeckResults

2493ef1

Inline non-self-descriptive fn infer_id

f5ad60c

Don't create a path with zero segments

bfcaf51

fmease force-pushed the rustdoc-ltd-improvs branch from 79a5881 to 96b43c6 Compare May 13, 2026 01:51

Don't try to resolve type-dependent paths in anon consts

e205784

fmease force-pushed the rustdoc-ltd-improvs branch from 96b43c6 to e205784 Compare May 13, 2026 01:56

fmease added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 13, 2026

		@@ -653,6 +653,15 @@ add the `--scrape-tests` flag.
		This flag enables the generation of links in the source code pages which allow the reader
		to jump to a type definition.

	//@ has - '//a[@href="{{channel}}/core/fmt/macros/derive.Debug.html"]' 'Debug'
	//@ has - '//a[@href="{{channel}}/core/cmp/derive.PartialEq.html"]' 'PartialEq'

Uh oh!

Conversation

fmease commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented May 10, 2026

Uh oh!

This comment has been minimized.

fmease May 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease commented May 12, 2026

Uh oh!

fmease May 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease May 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented May 12, 2026

Uh oh!

fmease commented May 12, 2026

Uh oh!

rust-bors Bot commented May 12, 2026

Uh oh!

fmease commented May 13, 2026

Uh oh!

fmease commented May 13, 2026

Uh oh!

rust-bors Bot commented May 13, 2026

Uh oh!

rustbot commented May 13, 2026

Uh oh!

rust-bors Bot commented May 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fmease commented May 10, 2026 •

edited

Loading

fmease May 12, 2026 •

edited by rustbot

Loading

fmease May 12, 2026 •

edited by rustbot

Loading

fmease May 12, 2026 •

edited by rustbot

Loading

fmease May 12, 2026 •

edited by rustbot

Loading

fmease May 12, 2026 •

edited by rustbot

Loading