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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. 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.

5 participants