Skip to content
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

Remove Rvalue::CheckedBinaryOp #125173

Merged
merged 2 commits into from
May 20, 2024
Merged

Remove Rvalue::CheckedBinaryOp #125173

merged 2 commits into from
May 20, 2024

Conversation

scottmcm
Copy link
Member

@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 May 16, 2024
@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm force-pushed the never-checked branch 2 times, most recently from 7070071 to abce8c5 Compare May 16, 2024 09:39
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please :)

I only looked at the interpreter-related parts and the general MIR syntax.

I think further cleanup should be possible, e.g. in the interpreter I don't think we want to even still have binop_{with,ignoring}_overflow, and maybe codegen also can be simplified, but that can be done on follow-up PRs.

compiler/rustc_middle/src/mir/syntax.rs Outdated Show resolved Hide resolved
let b = vals.1.ty(&self.body.local_decls, self.tcx);
match op {
Add | Sub | Mul => {
AddWithOverflow | SubWithOverflow | MulWithOverflow => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be merged with AddUnchecked | SubUnchecked | MulUnchecked | Shl | ShlUnchecked | Shr | ShrUnchecked? They all have the same requirements I think -- having two ints.

@@ -1088,7 +1081,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "unequal types" thing is already checked up above the match.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point -- with it being in the same block it gets the binop_right_homogeneous check already.

@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm marked this pull request as ready for review May 18, 2024 04:39
@rustbot
Copy link
Collaborator

rustbot commented May 18, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in run-make tests.

cc @jieyouxu

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

@scottmcm
Copy link
Member Author

Wow that's alot of pings 😬 Hi everyone 👋

I think this is ready to go now.
r? mir

There's lots of followups we can do, but I think it's make sense to land the MIR syntax change first, since it affects so much stuff and has to land at once. Then we can, as Ralf mentioned above (#125173 (review)), do individual PRs for cleanup of different parts (CTFE, codegen, mir-opts, …).

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 20, 2024

📌 Commit d83c65e has been approved by davidtwco

It is now in the queue for this repository.

@bors 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 May 20, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#124682 (Suggest setting lifetime in borrowck error involving types with elided lifetimes)
 - rust-lang#124917 (Check whether the next_node is else-less if in get_return_block)
 - rust-lang#125106 (coverage: Memoize and simplify counter expressions)
 - rust-lang#125173 (Remove `Rvalue::CheckedBinaryOp`)
 - rust-lang#125305 (add some codegen tests for issue 120493)
 - rust-lang#125314 ( Add an experimental feature gate for global registration)
 - rust-lang#125318 (Migrate `run-make/rustdoc-scrape-examples-whitespace` to `rmake.rs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9987e90 into rust-lang:master May 20, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2024
Rollup merge of rust-lang#125173 - scottmcm:never-checked, r=davidtwco

Remove `Rvalue::CheckedBinaryOp`

Zulip conversation: <https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/intrinsics.20vs.20binop.2Funop/near/438729996>
cc `@RalfJung`

While it's a draft,
r? ghost
@scottmcm scottmcm deleted the never-checked branch May 21, 2024 01:06
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
…r=<try>

interpret: make overflowing binops just normal binops

Follow-up to rust-lang#125173 (Cc `@scottmcm)`
bors added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
…r=oli-obk

interpret: make overflowing binops just normal binops

Follow-up to rust-lang#125173 (Cc `@scottmcm)`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 24, 2024
interpret: make overflowing binops just normal binops

Follow-up to rust-lang/rust#125173 (Cc `@scottmcm)`
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 24, 2024
interpret: make overflowing binops just normal binops

Follow-up to rust-lang/rust#125173 (Cc `@scottmcm)`
bors pushed a commit to rust-lang-ci/rust that referenced this pull request May 30, 2024
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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