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

[arithmetic_side_effects] Fix #10252 #10309

Merged
merged 1 commit into from
Mar 9, 2023
Merged

[arithmetic_side_effects] Fix #10252 #10309

merged 1 commit into from
Mar 9, 2023

Conversation

c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented Feb 9, 2023

Fix #10252

At least for integers, shifts are already handled by the compiler.


changelog: [arithmetic_side_effects]: No longer lints on right or left shifts with constant integers, as the compiler warns about them.
#10309

@rustbot
Copy link
Collaborator

rustbot commented Feb 9, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 9, 2023
@xFrednet
Copy link
Member

Hey @c410-f3r regarding the changelog entry, would you mind telling me what CTFT stands for?

Based on the linked issue, I understand that this change allows shifts, if they're verifiably inside the bounds of the shifted integer. Is that correct?

@c410-f3r
Copy link
Contributor Author

Hey @c410-f3r regarding the changelog entry, would you mind telling me what CTFT stands for?

CTFE -> Constant Time Function Evaluator

Based on the linked issue, I understand that this change allows shifts, if they're verifiably inside the bounds of the shifted integer. Is that correct?

Yes, Rustc is smart enough to handle out-of-bounds shifts for known integers.

Actually, the term CTFE is being abused and I am not totally sure if this specific part is indeed handled by the CTFE. It is probably best to just say compiler or Rustc instead.

@bors
Copy link
Collaborator

bors commented Feb 12, 2023

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

@xFrednet
Copy link
Member

Thank you for the explanation! :)

@c410-f3r
Copy link
Contributor Author

ping @giraffate

@giraffate
Copy link
Contributor

I'll review this in a few days.

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Mar 9, 2023

📌 Commit c439373 has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Mar 9, 2023

⌛ Testing commit c439373 with merge ea4ebed...

@bors
Copy link
Collaborator

bors commented Mar 9, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing ea4ebed to master...

@bors bors merged commit ea4ebed into rust-lang:master Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[arithmetic_side_effects] Shifts by in-bounds literals trigger a false positive.
5 participants