Implement feature integer_casts

[orlp]

View all comments

Tracking issue: #157388

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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: libs
• libs expanded to 8 candidates
• Random selection from Mark-Simulacrum, jhpratt

[jhpratt]

r? jhpratt

@rustbot author

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[orlp]

@rustbot ready

[jhpratt]

That wording works for me.

@bors r+ rollup

[rust-bors]

📌 Commit 9d5fa40 has been approved by jhpratt

It is now in the queue for this repository.

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[jieyouxu]

@bors p=101
@bors retry

[rust-bors]

⌛ Testing commit d79b87b with merge 47e54da...

Workflow: https://github.com/rust-lang/rust/actions/runs/27116431268

[jhpratt]

@bors treeclosed-

@bors yield

Yielding so that the rollup=nevers get a chance to run before things get busier with the start of business hours in Europe. An n=4 rollup doesn't make much sense when there's 3 nevers waiting.

[rust-bors]

Tree is now open for merging.

[rust-bors]

Auto build was cancelled. Cancelled workflows:

• https://github.com/rust-lang/rust/actions/runs/27116431268

The next pull request likely to be tested is #157402.

[rust-bors]

⌛ Testing commit d79b87b with merge e6f8fdb...

Workflow: https://github.com/rust-lang/rust/actions/runs/27116808211

[jhpratt]

@bors p=0

@bors yield

[rust-bors]

Auto build was cancelled. Cancelled workflows:

• https://github.com/rust-lang/rust/actions/runs/27116808211

The next pull request likely to be tested is #155607.

[Mark-Simulacrum]

@rust-timer build d65ab4d

cc #157600 (comment)

[rust-timer]

Finished benchmarking commit (d65ab4d): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never
@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.6%	[0.2%, 1.6%]	16
Regressions ❌ (secondary)	1.1%	[0.3%, 1.8%]	24
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.3%]	4
Improvements ✅ (secondary)	-0.4%	[-1.0%, -0.2%]	7
All ❌✅ (primary)	0.4%	[-0.5%, 1.6%]	20

Max RSS (memory usage)

Results (primary 1.3%, secondary 3.6%)

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

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.0%, 1.5%]	5
Regressions ❌ (secondary)	3.6%	[3.6%, 3.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[1.0%, 1.5%]	5

Cycles

Results (secondary -2.8%)

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)	1.8%	[1.8%, 1.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-6.1%, -2.3%]	7
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 516.268s -> 516.499s (0.04%)
Artifact size: 400.81 MiB -> 400.82 MiB (0.00%)

[JonathanBrouwer]

Caused the perf regression in #157600
Is there anything we can do about this?

[jhpratt]

I'm unsure how it could cause such a significant change in the first place, to be honest.

[orlp]

@jhpratt @JonathanBrouwer I think it's fairly obvious. All the regressions are for creating docs. This PR adds ~288 trait implementations to the standard library that end up being documented/processed.

If the trait ends up being perma-unstable I can see us #[doc(hidden)] them, but if the goal is to eventually stabilize them as well (so you can write e.g. u8::checked_cast_from), I don't think there's much we can do.

[JonathanBrouwer]

Does adding trait impls to the stdlib also make downstream crates slower to document? I guess that makes sense to some degree but still interesting

[orlp]

Yes, but also not by much. Adding 288 new impls is quite a lot and only resulted in 0.3-1% slowdowns, with 1.5% being an exception on hello world. The latter crate naturally is the fastest to document in the first place, meaning any fixed overhead shows up more.

It's not my decision but I don't think further action is required at this time.

[JonathanBrouwer]

I agree, but I'll cc the rustdoc team so they at least know about it

Cc @rust-lang/rustdoc