Skip to content

tidy: document u64 limit for numeric sort keys#159273

Open
kn1g78 wants to merge 2 commits into
rust-lang:mainfrom
kn1g78:tidy-handle-large-numeric-sorts
Open

tidy: document u64 limit for numeric sort keys#159273
kn1g78 wants to merge 2 commits into
rust-lang:mainfrom
kn1g78:tidy-handle-large-numeric-sorts

Conversation

@kn1g78

@kn1g78 kn1g78 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

The tidy-alphabetical checker uses natural sorting for numeric sequences, parsing each sequence as u64.

Document this limit so that numeric sequences in marked lists remain within the supported range.

Tests:

  • cargo test --offline --manifest-path src/tools/tidy/Cargo.toml
  • x test tidy

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 14, 2026
@kn1g78 kn1g78 marked this pull request as ready for review July 15, 2026 13:20
@rustbot 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 Jul 15, 2026
@rustbot

rustbot commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

r? @clubby789

rustbot has assigned @clubby789.
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: bootstrap
  • bootstrap expanded to 6 candidates
  • Random selection from Mark-Simulacrum, clubby789, jieyouxu

@clubby789

Copy link
Copy Markdown
Contributor

Why the switch to ASCII?

@kn1g78

kn1g78 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Why the switch to ASCII?

is_numeric also recognizes Unicode numeric characters, while u64::parse only accepts ASCII digits. Using is_ascii_digit keeps the numeric-run detection consistent with the supported numeric syntax and avoids treating Unicode numerals as numeric runs. Supporting Unicode numeric sorting would require separate handling.

@Kivooeo

Kivooeo commented Jul 15, 2026

Copy link
Copy Markdown
Member

which panicked when a marked list contained a number larger than u64::MAX

how often in practice this happens?

@kn1g78

kn1g78 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

which panicked when a marked list contained a number larger than u64::MAX

how often in practice this happens?

this does not currently happen in practice and is unlikely with the existing use cases. This PR is defensive hardening for an undocumented u64 limit in the generic comparator, rather than a fix for an observed failure.

@Kivooeo

Kivooeo commented Jul 15, 2026

Copy link
Copy Markdown
Member

let's just document u64 limit then

@kn1g78 kn1g78 changed the title tidy: handle large numeric sort keys tidy: document u64 limit for numeric sort keys Jul 15, 2026
@kn1g78

kn1g78 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

let's just document u64 limit then

Done. Thank you!

@clubby789

clubby789 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Please update the PR description. In future, please don't use AI to generate PR descriptions
@bors r+ rollup

@rust-bors

rust-bors Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

📌 Commit d4dff69 has been approved by clubby789

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 Jul 15, 2026
@workingjubilee

workingjubilee commented Jul 15, 2026

Copy link
Copy Markdown
Member

@bors r-

It should have the PR updated for its description first, before approval, because when a rollup is made, that will take the PR description at that moment, not whatever edits are landed. That PR description then becomes a commit message.

@rust-bors 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 Jul 15, 2026
@rust-bors

rust-bors Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

This pull request was unapproved.

View changes since this unapproval

@kn1g78

kn1g78 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

done.

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

Labels

A-tidy Area: The tidy tool S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants