-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CTFE: tweak aggregate rvalue handling #89370
Conversation
Some changes occured to the CTFE / Miri engine cc @rust-lang/miri |
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
r? @oli-obk Also making sure the extra assertions do not regress performance... this is not the array case though so I think we are good. |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 268bb46 with merge e1cc07f9bc16c8a10f1a8941bc2a03231bcc1c50... |
☀️ Try build successful - checks-actions |
Queued e1cc07f9bc16c8a10f1a8941bc2a03231bcc1c50 with parent 50f9f78, future comparison URL. |
Finished benchmarking commit (e1cc07f9bc16c8a10f1a8941bc2a03231bcc1c50): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. @bors rollup=never |
@bors r+ rollup |
📌 Commit 268bb46 has been approved by |
…li-obk CTFE: tweak aggregate rvalue handling I have not looked at this code in ages... I think Miri does not even hit it, since (most?) aggregate rvalues are lowered somewhere in the MIR pipeline, but CTFE does hit it. So this adds some extra sanity assertions, and removes a ZST special case -- ZST should only be special cased fairly late (when the actual memory access happens); e.g. `!` is a ZST and we still want `copy_op` to be called for it since it will perform validation (and raise UB, since `!` is never valid).
…li-obk CTFE: tweak aggregate rvalue handling I have not looked at this code in ages... I think Miri does not even hit it, since (most?) aggregate rvalues are lowered somewhere in the MIR pipeline, but CTFE does hit it. So this adds some extra sanity assertions, and removes a ZST special case -- ZST should only be special cased fairly late (when the actual memory access happens); e.g. `!` is a ZST and we still want `copy_op` to be called for it since it will perform validation (and raise UB, since `!` is never valid).
…li-obk CTFE: tweak aggregate rvalue handling I have not looked at this code in ages... I think Miri does not even hit it, since (most?) aggregate rvalues are lowered somewhere in the MIR pipeline, but CTFE does hit it. So this adds some extra sanity assertions, and removes a ZST special case -- ZST should only be special cased fairly late (when the actual memory access happens); e.g. `!` is a ZST and we still want `copy_op` to be called for it since it will perform validation (and raise UB, since `!` is never valid).
…arth Rollup of 8 pull requests Successful merges: - rust-lang#88782 (Fix ICE when `start` lang item has wrong generics) - rust-lang#89202 (Resolve infered types when complaining about unexpected call type ) - rust-lang#89248 (Suggest similarly named associated items in trait impls) - rust-lang#89303 (Add `#[must_not_suspend]` to some types in std) - rust-lang#89306 (thread: implements available_concurrency on haiku) - rust-lang#89314 (fix(lint): don't suggest refutable patterns to "fix" irrefutable bind) - rust-lang#89370 (CTFE: tweak aggregate rvalue handling) - rust-lang#89392 (bootstrap: Update comment in config.library.toml.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
I have not looked at this code in ages... I think Miri does not even hit it, since (most?) aggregate rvalues are lowered somewhere in the MIR pipeline, but CTFE does hit it.
So this adds some extra sanity assertions, and removes a ZST special case -- ZST should only be special cased fairly late (when the actual memory access happens); e.g.
!
is a ZST and we still wantcopy_op
to be called for it since it will perform validation (and raise UB, since!
is never valid).