Skip to content

rustdoc: do not include extra stuff in span#157561

Open
notriddle wants to merge 4 commits into
rust-lang:mainfrom
notriddle:do-not-destroy-function-name
Open

rustdoc: do not include extra stuff in span#157561
notriddle wants to merge 4 commits into
rust-lang:mainfrom
notriddle:do-not-destroy-function-name

Conversation

@notriddle

@notriddle notriddle commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

View all comments

This change prevents our lints from returning a span with stuff in it that isn't actually part of the doc comment. When that happens, it returns None instead.

Fixes rust-lang/rust-clippy#16169, since the bug was reported against the clippy::doc_paragraphs_missing_punctuation lint but is actually a bug in source_span_for_markdown_range_inner.

@rustbot 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 Jun 7, 2026
@rustbot

rustbot commented Jun 7, 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

Copy link
Copy Markdown
Contributor Author

p.s. this change is a lot easier to understand if you use the hide whitespace setting on the diff.

@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle force-pushed the do-not-destroy-function-name branch from d2784cd to 9af61c5 Compare June 7, 2026 05:52
@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle force-pushed the do-not-destroy-function-name branch from 9af61c5 to 7db2845 Compare June 7, 2026 07:05
@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle force-pushed the do-not-destroy-function-name branch from 7db2845 to 9d216d1 Compare June 7, 2026 15:26
@rust-log-analyzer

This comment has been minimized.

@rustbot

rustbot commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

The Clippy subtree was changed

cc @rust-lang/clippy

@notriddle notriddle force-pushed the do-not-destroy-function-name branch from 31c9e0e to d13270c Compare June 8, 2026 02:23
@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Jun 8, 2026
@notriddle notriddle requested a review from lolbinarycat June 8, 2026 04:07
@rust-bors

This comment has been minimized.

This change prevents our lints from returning a span with stuff in it
that isn't actually part of the doc comment. When that happens, it
returns `None` instead.
@notriddle notriddle force-pushed the do-not-destroy-function-name branch from d13270c to 33fb30e Compare June 12, 2026 16:54
@rustbot

rustbot commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

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.

@lolbinarycat

Copy link
Copy Markdown
Contributor

Do we need to be optimizing our diagnostic code (cold path) to the extent of using binary search to find an item in a usually small list? this only speeds up doc generation if a crate has a ton of warnings. I guess I'll see if it shows up at all in a perf run.

@bors try @rust-timer queue profiles=doc

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 13, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 13, 2026
rustdoc: do not include extra stuff in span
@rust-bors

rust-bors Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: a64b630 (a64b630f04e7c6ae84c88850e738cae44ce235d7, parent: 65407954098ca3c19f0d46092cb374b5d3e9dc3c)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (a64b630): comparison URL.

Overall result: no relevant changes - 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

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 518.069s -> 524.961s (1.33%)
Artifact size: 401.35 MiB -> 401.38 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 13, 2026
@notriddle

Copy link
Copy Markdown
Contributor Author

Do we need to be optimizing our diagnostic code (cold path) to the extent of using binary search to find an item in a usually small list?

I knew it would be O(n2) if just did a linear scan. Even in the cold path, I'd prefer to avoid that.

@lolbinarycat

Copy link
Copy Markdown
Contributor

I don't think algorimic complexity is really relevant here.
Screenshot_2026-06-13-11-07-07-21_3aea4af51f236e4932235fdada7d1643

@rust-log-analyzer

This comment has been minimized.

It requires an allocation, and doesn't seem to help in practice.
Fixes a nit found during review.
@notriddle notriddle force-pushed the do-not-destroy-function-name branch from 93b65a3 to ae2edb4 Compare June 13, 2026 17:23
@notriddle

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue profiles=doc

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 13, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 13, 2026
rustdoc: do not include extra stuff in span
@rust-bors

rust-bors Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 1a5fffe (1a5fffe9955796b4c20810d806e19fca4d8f6e34, parent: e7ef22d23f10e9e05546592aa9c1e3e8bb288969)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (1a5fffe): comparison URL.

Overall result: no relevant changes - 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

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 521.088s -> 525.76s (0.90%)
Artifact size: 401.44 MiB -> 401.46 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 14, 2026
@notriddle

Copy link
Copy Markdown
Contributor Author

Here's the new non-relevant result set:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc-paragraphs-missing-punctuation destroys function name

5 participants