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

Implement Integer methods for Wrapping #48810

Merged
merged 3 commits into from
Mar 20, 2018

Conversation

Phlosioneer
Copy link
Contributor

Wrapping now implements:

count_ones, count_zeros, leading_zeros,
trailing_zeros, rotate_left, rotate_right, swap_bytes, from_be,
from_le, to_be, to_le, and pow

where T is:

u8, u16, u32, u64, usize, i8, i16, i32, i64, or isize.

Docs were written for all these methods, as well as examples. The
examples mirror the ones on u8, u16, etc... for consistency.

Closes #32463

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @BurntSushi (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 7, 2018
@Phlosioneer
Copy link
Contributor Author

Because of the last comment on #32463, I think dtolnay is probably the best person to review this.

r? @dtolnay

@rust-highfive rust-highfive assigned dtolnay and unassigned BurntSushi Mar 7, 2018
#[inline]
#[unstable(feature = "wrapping_int_impl", issue = "32463")]
pub fn pow(self, exp: u32) -> Self {
Wrapping(self.0.pow(exp))
Copy link
Member

@scottmcm scottmcm Mar 7, 2018

Choose a reason for hiding this comment

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

I think this one should call wrapping_pow? And maybe include a doctest demonstrating that it wraps...

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 wanted to name it pow to mirror the names on u8 etc. These impls are intended to make generic code that operates on integer types work seamlessly with wrapping integer types, and that requires the method names to be the same.

I'll add a wrapping example to the doctest.

Copy link
Member

Choose a reason for hiding this comment

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

I think @scottmcm meant that within, it should call self.0.wrapping_pow(exp). Otherwise, it might actually overflow / panic.

@@ -355,3 +651,6 @@ mod shift_max {
pub const u64: u32 = i64;
pub use self::platform::usize;
}



Copy link
Member

Choose a reason for hiding this comment

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

[00:06:04] tidy error: /checkout/src/libcore/num/wrapping.rs:528: trailing whitespace
[00:06:04] tidy error: /checkout/src/libcore/num/wrapping.rs: too many trailing newlines (4)

@Phlosioneer Phlosioneer force-pushed the 32463-impl-integer-for-wrapping branch from afb1870 to 02aa39f Compare March 8, 2018 07:36
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Mar 10, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Mar 10, 2018

📌 Commit 910d855 has been approved by dtolnay

@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 10, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 10, 2018
…wrapping, r=dtolnay

Implement Integer methods for Wrapping

Wrapping<T> now implements:

count_ones, count_zeros, leading_zeros,
trailing_zeros, rotate_left, rotate_right, swap_bytes, from_be,
from_le, to_be, to_le, and pow

where T is:

u8, u16, u32, u64, usize, i8, i16, i32, i64, or isize.

Docs were written for all these methods, as well as examples. The
examples mirror the ones on u8, u16, etc... for consistency.

Closes rust-lang#32463
@kennytm
Copy link
Member

kennytm commented Mar 11, 2018

@bors rollup

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Mar 12, 2018
…wrapping, r=dtolnay

Implement Integer methods for Wrapping

Wrapping<T> now implements:

count_ones, count_zeros, leading_zeros,
trailing_zeros, rotate_left, rotate_right, swap_bytes, from_be,
from_le, to_be, to_le, and pow

where T is:

u8, u16, u32, u64, usize, i8, i16, i32, i64, or isize.

Docs were written for all these methods, as well as examples. The
examples mirror the ones on u8, u16, etc... for consistency.

Closes rust-lang#32463
@kennytm
Copy link
Member

kennytm commented Mar 13, 2018

@bors rollup-

@bors
Copy link
Contributor

bors commented Mar 14, 2018

⌛ Testing commit 910d855 with merge 2b0dbffed5e11b4b565e4011d77ccf12493bd82b...

@bors
Copy link
Contributor

bors commented Mar 14, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 14, 2018
@Phlosioneer
Copy link
Contributor Author

Is there something wrong with my PR...? A rollup error log doesn't usually show up directly under a PR. The error is weird, though... something about duplicate rustdoc for impl<T> Send for Outer<T> where T: Copy + Send?

@scottmcm
Copy link
Member

@Phlosioneer It was removed from the rollup, see #48810 (comment)

(I don't understand the error either.)

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 14, 2018
@kennytm kennytm removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 14, 2018
@Phlosioneer
Copy link
Contributor Author

Phlosioneer commented Mar 16, 2018

uhm, what do I need to do? I don't think this is waiting-on-author...

@kennytm
Copy link
Member

kennytm commented Mar 16, 2018

@Phlosioneer Try to change rustdoc/synthetic_auto/no-redundancy.rs to match either Copy + Send or Send + Copy. You can use @matches with a regular expression, see htmldocck.py for syntax. I suspect this is the same issue as #48265 (review).

(cc @GuillaumeGomez about the order change on i686.)

@SimonSapin
Copy link
Contributor

I’ve filed #49123 for that rustdoc test failure.

Wrapping<T> now implements:

count_ones, count_zeros, leading_zeros,
trailing_zeros, rotate_left, rotate_right, swap_bytes, from_be,
from_le, to_be, to_le, and pow

where T is:

u8, u16, u32, u64, usize, i8, i16, i32, i64, or isize.

Docs were written for all these methods, as well as examples. The
examples mirror the ones on u8, u16, etc... for consistency.

Closes rust-lang#32463
@Phlosioneer Phlosioneer force-pushed the 32463-impl-integer-for-wrapping branch from 910d855 to bf101a5 Compare March 19, 2018 05:40
@Phlosioneer
Copy link
Contributor Author

I rebased against master, and I don't see #49123 during local testing. We'll see if travis agrees.

@GuillaumeGomez
Copy link
Member

I have no idea why there is a change in the order... If there is a problem, I think it comes from the internals of rust instead of rustdoc.

@kennytm
Copy link
Member

kennytm commented Mar 19, 2018

@bors r=dtolnay

Let's try again then.

@bors
Copy link
Contributor

bors commented Mar 19, 2018

📌 Commit bf101a5 has been approved by dtolnay

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 19, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 19, 2018
…wrapping, r=dtolnay

Implement Integer methods for Wrapping

Wrapping<T> now implements:

count_ones, count_zeros, leading_zeros,
trailing_zeros, rotate_left, rotate_right, swap_bytes, from_be,
from_le, to_be, to_le, and pow

where T is:

u8, u16, u32, u64, usize, i8, i16, i32, i64, or isize.

Docs were written for all these methods, as well as examples. The
examples mirror the ones on u8, u16, etc... for consistency.

Closes rust-lang#32463
@kennytm
Copy link
Member

kennytm commented Mar 20, 2018

@bors rollup

bors added a commit that referenced this pull request Mar 20, 2018
Rollup of 17 pull requests

- Successful merges: #46518, #48810, #48834, #48902, #49004, #49092, #49096, #49099, #49104, #49125, #49139, #49152, #49157, #49161, #49166, #49176, #49184
- Failed merges:
@bors bors merged commit bf101a5 into rust-lang:master Mar 20, 2018
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.

10 participants