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

Reduces Code Repetitions like !n >> amt #59101

Merged
merged 4 commits into from
Mar 13, 2019

Conversation

kenta7777
Copy link
Contributor

@kenta7777 kenta7777 commented Mar 11, 2019

Fixes #49937 .
This PR contains defining a function which operates bit inversion and reducing bit operation like !0u128 >> (128 - size.bits()).

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 11, 2019
@kenta7777 kenta7777 changed the title Reduce Code epetition Reduce Code Repetition Mar 11, 2019
@kenta7777 kenta7777 changed the title Reduce Code Repetition Reduces Code Repetitions like !n >> amt Mar 11, 2019
@Centril
Copy link
Contributor

Centril commented Mar 11, 2019

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned pnkfelix Mar 11, 2019
@@ -115,14 +116,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
ty::Int(ity) => {
// FIXME(49937): refactor these bit manipulations into interpret.
let size = Integer::from_attr(&tcx, SignedInt(ity)).size();
let max = !0u128 >> (128 - size.bits());
let max = mask(size);
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be truncate(u128::max_value(), size)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review. Then, should I replace mask with truncate and remove the definition of mask?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, I think that's for the best, unless you see a lot of other sites in the compiler where using mask would make sense.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 11, 2019

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Mar 11, 2019

📌 Commit 18b40c6 has been approved by oli-obk

@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 Mar 11, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Mar 11, 2019

@bors rollup

kennytm added a commit to kennytm/rust that referenced this pull request Mar 11, 2019
…=oli-obk

Reduces Code Repetitions like `!n >> amt`

Fixes rust-lang#49937 .
This PR contains defining a function which operates bit inversion and reducing bit operation like `!0u128 >> (128 - size.bits())`.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Mar 12, 2019
…=oli-obk

Reduces Code Repetitions like `!n >> amt`

Fixes rust-lang#49937 .
This PR contains defining a function which operates bit inversion and reducing bit operation like `!0u128 >> (128 - size.bits())`.
Centril added a commit to Centril/rust that referenced this pull request Mar 13, 2019
…=oli-obk

Reduces Code Repetitions like `!n >> amt`

Fixes rust-lang#49937 .
This PR contains defining a function which operates bit inversion and reducing bit operation like `!0u128 >> (128 - size.bits())`.
bors added a commit that referenced this pull request Mar 13, 2019
Rollup of 16 pull requests

Successful merges:

 - #58829 (librustc_interface: Update scoped-tls to 1.0)
 - #58876 (Parse lifetimes that start with a number and give specific error)
 - #58908 (Update rand version)
 - #58998 (Fix documentation of from_ne_bytes and from_le_bytes)
 - #59056 (Use lifetime contravariance to elide more lifetimes in core+alloc+std)
 - #59057 (Standardize `Range*` documentation)
 - #59080 (Fix incorrect links in librustc_codegen_llvm documentation)
 - #59083 (Fix #54822 and associated faulty tests)
 - #59093 (Remove precompute_in_scope_traits_hashes)
 - #59101 (Reduces Code Repetitions like `!n >> amt`)
 - #59121 (impl FromIterator for Result: Use assert_eq! instead of assert!)
 - #59124 (Replace assert with assert_eq)
 - #59129 (Visit impl Trait for dead_code lint)
 - #59130 (Note that NonNull does not launder shared references for mutation)
 - #59132 (ignore higher-ranked object bound conditions created by WF)
 - #59138 (Simplify Iterator::{min, max})

Failed merges:

r? @ghost
@bors bors merged commit 18b40c6 into rust-lang:master Mar 13, 2019
@kenta7777
Copy link
Contributor Author

Thanks!

@kenta7777 kenta7777 deleted the reduce-code-repetition branch March 13, 2019 06:38
@@ -115,14 +116,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
ty::Int(ity) => {
// FIXME(49937): refactor these bit manipulations into interpret.
Copy link
Member

Choose a reason for hiding this comment

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

These FIXMEs look resolved now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for reporting. I forgot to remove this comment out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think let bias = 1u128 << (size.bits() - 1); is not resolved.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants