Skip to content

remove scalar size mismatch interpreter error, make it an ICE instead - #161238

Merged
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
RalfJung:remove-scalar-size-mismatch
Aug 18, 2026
Merged

remove scalar size mismatch interpreter error, make it an ICE instead#161238
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
RalfJung:remove-scalar-size-mismatch

Conversation

@RalfJung

@RalfJung RalfJung commented Aug 17, 2026

Copy link
Copy Markdown
Member

This error was added in #71569 to deal with ICEs on incorrect Miri shim signatures. However, that was a partial fix: if the signature is "even more incorrect", using a non-scalar type where the type should be scalar, we still ICE. The proper fix is tracked in rust-lang/miri#3842, and that is already used for a good chunk of our shims.

I don't think it's worth keeping around the old, incomplete check here, so let's get rid of it.

Sadly, the to_$int methods on Scalar still return a Result as they must still error if the scalar is a pointer and we are in const-eval where we cannot turn that pointer into an integer. We should probably use ScalarInt in a lot more places where we currently use Scalar to statically exclude this case... but that's a change for another time.

@rustbot

rustbot commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@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. labels Aug 17, 2026
@rustbot

rustbot commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

r? @petrochenkov

rustbot has assigned @petrochenkov.
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: compiler, mir
  • compiler, mir expanded to 75 candidates
  • Random selection from 18 candidates

@petrochenkov

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 93c99eb has been approved by petrochenkov

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 Aug 18, 2026
rust-bors Bot pushed a commit that referenced this pull request Aug 18, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - #161024 (resolving cyclic glob vis-max)
 - #161231 (passes: `rustc_scalable_vector` fields are not dead)
 - #161238 (remove scalar size mismatch interpreter error, make it an ICE instead)
 - #160345 (Resolver: add `checked` methods for `Cm(Ref)Cell`)
 - #161129 (Avoid ICE when recovering parenthesized type parameters)
 - #161235 (Compute job time in post-merge-report from the actual GitHub duration)
 - #161239 (Fix `#[repr(transparent)]` wrapper types not working with `Complex<T>`)
 - #161244 (Add regression test for indexing an unsized static without a body)
 - #161257 (Ignore target feature test when LLVM fails to compile minicore)
 - #161258 (perf: return early from in_external_macro for root contexts)
 - #161278 (Add regression test for normalization failure on erased closure in async block)
@rust-bors
rust-bors Bot merged commit 966b1ec into rust-lang:main Aug 18, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Aug 18, 2026
rust-bors Bot pushed a commit that referenced this pull request Aug 18, 2026
Rollup merge of #161238 - RalfJung:remove-scalar-size-mismatch, r=petrochenkov

remove scalar size mismatch interpreter error, make it an ICE instead

This error was added in #71569 to deal with ICEs on incorrect Miri shim signatures. However, that was a partial fix: if the signature is "even more incorrect", using a non-scalar type where the type should be scalar, we still ICE. The proper fix is tracked in rust-lang/miri#3842, and that is already used for a good chunk of our shims.

I don't think it's worth keeping around the old, incomplete check here, so let's get rid of it.

Sadly, the `to_$int` methods on `Scalar` still return a `Result` as they must still error if the scalar is a pointer and we are in const-eval where we cannot turn that pointer into an integer. We should probably use `ScalarInt` in a lot more places where we currently use `Scalar` to statically exclude this case... but that's a change for another time.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Aug 19, 2026
…uwer

Rollup of 11 pull requests

Successful merges:

 - rust-lang/rust#161024 (resolving cyclic glob vis-max)
 - rust-lang/rust#161231 (passes: `rustc_scalable_vector` fields are not dead)
 - rust-lang/rust#161238 (remove scalar size mismatch interpreter error, make it an ICE instead)
 - rust-lang/rust#160345 (Resolver: add `checked` methods for `Cm(Ref)Cell`)
 - rust-lang/rust#161129 (Avoid ICE when recovering parenthesized type parameters)
 - rust-lang/rust#161235 (Compute job time in post-merge-report from the actual GitHub duration)
 - rust-lang/rust#161239 (Fix `#[repr(transparent)]` wrapper types not working with `Complex<T>`)
 - rust-lang/rust#161244 (Add regression test for indexing an unsized static without a body)
 - rust-lang/rust#161257 (Ignore target feature test when LLVM fails to compile minicore)
 - rust-lang/rust#161258 (perf: return early from in_external_macro for root contexts)
 - rust-lang/rust#161278 (Add regression test for normalization failure on erased closure in async block)
@panstromek

Copy link
Copy Markdown
Contributor

for #161289

@rust-timer build 02877d8

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (02877d8): comparison URL.

Overall result: ❌ regressions - 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 rustc-perf
@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.5% [0.4%, 0.6%] 6
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [0.4%, 0.6%] 6

Max RSS (memory usage)

Results (primary -2.5%)

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

mean range count
Regressions ❌
(primary)
2.2% [2.2%, 2.2%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-7.2% [-7.2%, -7.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.5% [-7.2%, 2.2%] 2

Cycles

Results (primary 2.1%, secondary 5.5%)

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

mean range count
Regressions ❌
(primary)
2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)
5.5% [5.5%, 5.5%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Binary size

Results (secondary 0.0%)

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)
0.0% [0.0%, 0.0%] 19
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Bootstrap: 455.347s -> 456.26s (0.20%)
Artifact size: 398.98 MiB -> 399.05 MiB (0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Aug 21, 2026
@panstromek

panstromek commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Looks like this caused a regression in #161289

This intuitively seems like it should be an improvement instead, so it looks unexpected. Do we know where this regression is coming from? Can we do something about it?

@RalfJung

Copy link
Copy Markdown
Member Author

Indeed this is very strange, I can only guess the the inliner is making worse decisions in some places now?

@RalfJung
RalfJung deleted the remove-scalar-size-mismatch branch August 21, 2026 11:47
@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Aug 24, 2026
@Mark-Simulacrum

Copy link
Copy Markdown
Member

Going to mark this as triaged as the fix PR seems to be on track. Thanks for digging in!

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

Labels

perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants