Don't emit two `assume`s in transmutes when one is a subset of the other by scottmcm · Pull Request #144209 · rust-lang/rust

scottmcm · 2025-07-20T07:19:39Z

For example, transmuting between bool and Ordering doesn't need two assumes because one range is a superset of the other.

Multiple are still used for things like char <-> NonZero<u32>, which overlap but where neither fully contains the other.

rustbot · 2025-07-20T07:19:43Z

r? @lcnr

rustbot has assigned @lcnr.
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

rustbot · 2025-07-20T07:19:45Z

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

oli-obk · 2025-07-20T12:01:27Z

@bors2 try @rust-timer queue

rust-bors · 2025-07-20T12:01:31Z

⌛ Trying commit dd170e8 with merge 4e27cc3…

To cancel the try build, run the command @bors2 try cancel.

Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

rust-bors · 2025-07-20T14:15:29Z

☀️ Try build successful (CI)
Build commit: 4e27cc3 (4e27cc328172a524edb4dd605053867dc92cddfa, parent: 81af9d45698a19183b8552079cbc7bf893fad1e5)

rust-timer · 2025-07-20T15:27:45Z

Finished benchmarking commit (4e27cc3): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.9%, secondary 4.6%)

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)	4.6%	[4.6%, 4.6%]	1
Improvements ✅ (primary)	-2.9%	[-3.1%, -2.7%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.9%	[-3.1%, -2.7%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 465.3s -> 465.355s (0.01%)
Artifact size: 374.58 MiB -> 374.59 MiB (0.00%)

lcnr

r=me

I haven't looked at the test annotations in detail and don't feel 100% comfortable approving these. The code changes make sense the tests seem about right from what I can tell 😅 😁

scottmcm · 2025-07-21T21:43:07Z

@dianqk Perhaps you could double-check that the test updates here seem reasonable to you?

(This PR is not urgent; don't feel pressured to get to it promptly.)

bors · 2025-07-22T23:07:12Z

☔ The latest upstream changes (presumably #144249) made this pull request unmergeable. Please resolve the merge conflicts.

dianqk

r=me also after a nit.

(This PR is not urgent; don't feel pressured to get to it promptly.)

Never mind, I never felt that. I was just giving an estimated time.

For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

scottmcm · 2025-07-23T18:23:39Z

@bors r=lcnr,dianqk rollup=maybe (no perf changes in the report, so don't think it needs the separate run)

bors · 2025-07-23T18:23:42Z

📌 Commit a93a9aa has been approved by lcnr,dianqk

It is now in the queue for this repository.

Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

Rollup of 16 pull requests Successful merges: - #142569 (Suggest clone in user-write-code instead of inside macro) - #143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - #143424 (clippy fix: rely on autoderef) - #143970 (Update core::mem::copy documentation) - #143979 (Test fixes for Arm64EC Windows) - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc) - #144200 (Tweak output for non-`Clone` values moved into closures) - #144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - #144225 (Don't special-case llvm.* as nounwind) - #144314 (Hint that choose_pivot returns index in bounds) - #144316 (bootstrap: Move musl-root fallback out of sanity check) - #144364 (Update `dlmalloc` dependency of libstd) - #144368 (resolve: Remove `Scope::CrateRoot`) - #144373 (remove deprecated Error::description in impls) - #144390 (Remove dead code and extend test coverage and diagnostics around it) - #144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) r? `@ghost` `@rustbot` modify labels: rollup

Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

Rollup of 17 pull requests Successful merges: - #142569 (Suggest clone in user-write-code instead of inside macro) - #143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - #143424 (clippy fix: rely on autoderef) - #143970 (Update core::mem::copy documentation) - #143979 (Test fixes for Arm64EC Windows) - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc) - #144200 (Tweak output for non-`Clone` values moved into closures) - #144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - #144225 (Don't special-case llvm.* as nounwind) - #144314 (Hint that choose_pivot returns index in bounds) - #144316 (bootstrap: Move musl-root fallback out of sanity check) - #144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - #144364 (Update `dlmalloc` dependency of libstd) - #144368 (resolve: Remove `Scope::CrateRoot`) - #144390 (Remove dead code and extend test coverage and diagnostics around it) - #144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) - #144424 (Allow setting `release-blog-post` label with rustbot) r? `@ghost` `@rustbot` modify labels: rollup

Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

Rollup of 16 pull requests Successful merges: - #142569 (Suggest clone in user-write-code instead of inside macro) - #143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - #143424 (clippy fix: rely on autoderef) - #143970 (Update core::mem::copy documentation) - #143979 (Test fixes for Arm64EC Windows) - #144160 (tests: debuginfo: Work around or disable broken tests on powerpc) - #144200 (Tweak output for non-`Clone` values moved into closures) - #144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - #144314 (Hint that choose_pivot returns index in bounds) - #144316 (bootstrap: Move musl-root fallback out of sanity check) - #144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - #144364 (Update `dlmalloc` dependency of libstd) - #144368 (resolve: Remove `Scope::CrateRoot`) - #144390 (Remove dead code and extend test coverage and diagnostics around it) - #144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) - #144424 (Allow setting `release-blog-post` label with rustbot) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 12 pull requests Successful merges: - #142569 (Suggest clone in user-write-code instead of inside macro) - #143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - #143424 (clippy fix: rely on autoderef) - #143970 (Update core::mem::copy documentation) - #143979 (Test fixes for Arm64EC Windows) - #144200 (Tweak output for non-`Clone` values moved into closures) - #144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - #144314 (Hint that choose_pivot returns index in bounds) - #144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - #144368 (resolve: Remove `Scope::CrateRoot`) - #144390 (Remove dead code and extend test coverage and diagnostics around it) - #144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #144209 - scottmcm:assume_less, r=lcnr,dianqk Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

Rollup of 12 pull requests Successful merges: - rust-lang/rust#142569 (Suggest clone in user-write-code instead of inside macro) - rust-lang/rust#143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - rust-lang/rust#143424 (clippy fix: rely on autoderef) - rust-lang/rust#143970 (Update core::mem::copy documentation) - rust-lang/rust#143979 (Test fixes for Arm64EC Windows) - rust-lang/rust#144200 (Tweak output for non-`Clone` values moved into closures) - rust-lang/rust#144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - rust-lang/rust#144314 (Hint that choose_pivot returns index in bounds) - rust-lang/rust#144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - rust-lang/rust#144368 (resolve: Remove `Scope::CrateRoot`) - rust-lang/rust#144390 (Remove dead code and extend test coverage and diagnostics around it) - rust-lang/rust#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) r? `@ghost` `@rustbot` modify labels: rollup

Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#142569 (Suggest clone in user-write-code instead of inside macro) - rust-lang#143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - rust-lang#143424 (clippy fix: rely on autoderef) - rust-lang#143970 (Update core::mem::copy documentation) - rust-lang#143979 (Test fixes for Arm64EC Windows) - rust-lang#144200 (Tweak output for non-`Clone` values moved into closures) - rust-lang#144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - rust-lang#144314 (Hint that choose_pivot returns index in bounds) - rust-lang#144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - rust-lang#144368 (resolve: Remove `Scope::CrateRoot`) - rust-lang#144390 (Remove dead code and extend test coverage and diagnostics around it) - rust-lang#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) r? `@ghost` `@rustbot` modify labels: rollup

Don't emit two `assume`s in transmutes when one is a subset of the other For example, transmuting between `bool` and `Ordering` doesn't need two `assume`s because one range is a superset of the other. Multiple are still used for things like `char` <-> `NonZero<u32>`, which overlap but where neither fully contains the other.

…iaskrgr Rollup of 12 pull requests Successful merges: - rust-lang#142569 (Suggest clone in user-write-code instead of inside macro) - rust-lang#143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - rust-lang#143424 (clippy fix: rely on autoderef) - rust-lang#143970 (Update core::mem::copy documentation) - rust-lang#143979 (Test fixes for Arm64EC Windows) - rust-lang#144200 (Tweak output for non-`Clone` values moved into closures) - rust-lang#144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - rust-lang#144314 (Hint that choose_pivot returns index in bounds) - rust-lang#144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - rust-lang#144368 (resolve: Remove `Scope::CrateRoot`) - rust-lang#144390 (Remove dead code and extend test coverage and diagnostics around it) - rust-lang#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 12 pull requests Successful merges: - rust-lang/rust#142569 (Suggest clone in user-write-code instead of inside macro) - rust-lang/rust#143401 (tests: Don't check for self-printed output in std-backtrace.rs test) - rust-lang/rust#143424 (clippy fix: rely on autoderef) - rust-lang/rust#143970 (Update core::mem::copy documentation) - rust-lang/rust#143979 (Test fixes for Arm64EC Windows) - rust-lang/rust#144200 (Tweak output for non-`Clone` values moved into closures) - rust-lang/rust#144209 (Don't emit two `assume`s in transmutes when one is a subset of the other) - rust-lang/rust#144314 (Hint that choose_pivot returns index in bounds) - rust-lang/rust#144340 (UI test suite clarity changes: Rename `tests/ui/SUMMARY.md` and update rustc dev guide on `error-pattern`) - rust-lang/rust#144368 (resolve: Remove `Scope::CrateRoot`) - rust-lang/rust#144390 (Remove dead code and extend test coverage and diagnostics around it) - rust-lang/rust#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned lcnr Jul 20, 2025

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. labels Jul 20, 2025

This comment has been minimized.
Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 20, 2025

This comment has been minimized.
Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 20, 2025

lcnr reviewed Jul 21, 2025
View reviewed changes

dianqk self-assigned this Jul 22, 2025

dianqk approved these changes Jul 23, 2025
View reviewed changes

Comment threadcompiler/rustc_abi/src/lib.rs Outdated

scottmcm force-pushed the assume_less branch from dd170e8 to a93a9aaCompare July 23, 2025 16:16

bors 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 23, 2025

fmease mentioned this pull request Jul 24, 2025
Rollup of 16 pull requests #144418
Closed

scottmcm mentioned this pull request Jul 24, 2025
Stop generating assumes for validity ranges #129027
Closed

fmease mentioned this pull request Jul 25, 2025
Rollup of 17 pull requests #144428
Closed

This was referenced Jul 25, 2025
Rollup of 16 pull requests #144436
Closed
Rollup of 12 pull requests #144440
Merged

bors merged commit f414e7a into rust-lang:masterJul 25, 2025
11 checks passed

rustbot added this to the 1.90.0 milestone Jul 25, 2025

scottmcm deleted the assume_less branch July 25, 2025 16:22

Uh oh!

Uh oh!

Conversation

scottmcm commented Jul 20, 2025

Uh oh!

rustbot commented Jul 20, 2025

Uh oh!

rustbot commented Jul 20, 2025

Uh oh!

oli-obk commented Jul 20, 2025

Uh oh!

This comment has been minimized.

rust-borsBot commented Jul 20, 2025

Uh oh!

rust-borsBot commented Jul 20, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 20, 2025

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

scottmcm commented Jul 21, 2025

Uh oh!

bors commented Jul 22, 2025

Uh oh!

dianqk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottmcm commented Jul 23, 2025

Uh oh!

bors commented Jul 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants