-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Miri and miri-related code contains repetitions of (n << amt) >> amt
#56233
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
|
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
You have accidental submodule changes. Please use |
r? @oli-obk |
(Oh, you have multiple commits, you'll have to use |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@eddyb Thank you for your advice. I updated submodules and pushed again. |
@kenta7777 When I said |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Pinging from triage @kenta7777 any updates on this? |
r? @oli-obk |
The implementation looks good to me. @kenta7777 can you do another rebase and remove the submodule changes from your second commit? You can |
☔ The latest upstream changes (presumably #56305) made this pull request unmergeable. Please resolve the merge conflicts. |
@oli-obk I apologize for the delay in replying to you. I'll revise my commits following your advice. |
712ed93
to
70e85ad
Compare
@kenta7777 looks like you got some commits from another PR in here again. This time around a git fetch origin
git rebase -i origin/master and removing all commits that are not yours should work |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Don't worry about it, everything is fixable with git! No need to open a new PR. Can you show me the output of |
This is an output of
|
Ok, great! So the following commands should get you back to business:
Remember to remove any commits that are not yours from the list that shows up |
@oli-obk Thank you for your advice. I rebased following your advice. After that, This output is the first three commit log.
But, This is the output of
|
You can now use |
36617d4
to
dcbd573
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Because of my some work, I'll work on this bug fix a few days later. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Ping from triage @kenta7777 have you been able to make any progress on this? |
No. I'm considering fixing this error, but I can't find the way how to do. |
src/librustc_lint/types.rs
Outdated
let bits = layout::Integer::from_attr(&cx.tcx, ity).size().bits(); | ||
let actually = (val << (128 - bits)) as i128 >> (128 - bits); | ||
let size = layout::Integer::from_attr(&cx.tcx, ity).size(); | ||
let actually = sign_extend(val, size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that sign_extend
returns a u128
which contains the bits of a i128
. It should suffice to cast the result to i128
.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@oli-obk Thank you for your review. I have fixed the issue. |
The code looks perfect now. Please do another rebase (that should get rid of the merge commits). Just running |
@oli-obk I just ran |
3dfed67
to
b80332e
Compare
@bors r+ |
📌 Commit b80332e has been approved by |
@bors rollup |
Miri and miri-related code contains repetitions of `(n << amt) >> amt` I reduced some code repetitions contains `(n << amt) >> amt`. This pull request is related to rust-lang#49937.
Rollup of 5 pull requests Successful merges: - #56233 (Miri and miri-related code contains repetitions of `(n << amt) >> amt`) - #57645 (distinguish "no data" from "heterogeneous" in ABI) - #57734 (Fix evaluating trivial drop glue in constants) - #57886 (Add suggestion for moving type declaration before associated type bindings in generic arguments.) - #57890 (Fix wording in diagnostics page) Failed merges: r? @ghost
I reduced some code repetitions contains
(n << amt) >> amt
.This pull request is related to #49937.