Skip to content

std: Move overflowing ops to inherent methods#30466

Merged
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:move-wrapping-and-fill-out
Jan 14, 2016
Merged

std: Move overflowing ops to inherent methods#30466
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:move-wrapping-and-fill-out

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

This commit migrates all of the methods on num::wrapping::OverflowingOps onto
inherent methods of the integer types. This also fills out some missing gaps in
the saturating and checked departments such as:

  • saturating_mul
  • checked_{neg,rem,shl,shr}

This is done in preparation for stabilization,

cc #27755

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

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

@alexcrichton

Copy link
Copy Markdown
Member Author

r? @aturon

Note that this doesn't deprecate the old wrappers, it just prepares everything for the upcoming "deprecation commit"

@rust-highfive rust-highfive assigned aturon and unassigned nikomatsakis Dec 18, 2015
@aturon

aturon commented Dec 21, 2015

Copy link
Copy Markdown
Contributor

Hm, this might be slightly premature given discussion here. Let's wait for some overall consensus before landing this change.

@bors

bors commented Dec 22, 2015

Copy link
Copy Markdown
Collaborator

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

Comment thread src/libcore/num/mod.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment isn't quite clear about what value is returned on overflow.

@aturon

aturon commented Dec 31, 2015

Copy link
Copy Markdown
Contributor

@alexcrichton There seems to be consensus around the issue, so I've reviewed the PR and left a few nits. Once these are resolved r=me.

@ollie27

ollie27 commented Jan 5, 2016

Copy link
Copy Markdown
Contributor

I wouldn't expect overflowing_neg to return true when given 0 for unsigned ints as -0 can be represented as an unsigned int: 0.

Also it looks like checked_neg is missing for unsigned ints.

This commit migrates all of the methods on `num::wrapping::OverflowingOps` onto
inherent methods of the integer types. This also fills out some missing gaps in
the saturating and checked departments such as:

* `saturating_mul`
* `checked_{neg,rem,shl,shr}`

This is done in preparation for stabilization,

cc rust-lang#27755
@alexcrichton alexcrichton force-pushed the move-wrapping-and-fill-out branch from a5e04c6 to 7eb7699 Compare January 12, 2016 05:40
@alexcrichton

Copy link
Copy Markdown
Member Author

@ollie27 ah good point about negating 0!

@aturon I've updated with hopefully a clarification about shifting and overflow with the mask, although it may still be a little unclear as it's somewhat technical, what do you think?

@aturon

aturon commented Jan 13, 2016

Copy link
Copy Markdown
Contributor

@bors: r+

@bors

bors commented Jan 13, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 7eb7699 has been approved by aturon

@bors

bors commented Jan 14, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 7eb7699 with merge 9f6917d...

bors added a commit that referenced this pull request Jan 14, 2016
This commit migrates all of the methods on `num::wrapping::OverflowingOps` onto
inherent methods of the integer types. This also fills out some missing gaps in
the saturating and checked departments such as:

* `saturating_mul`
* `checked_{neg,rem,shl,shr}`

This is done in preparation for stabilization,

cc #27755
@bors bors merged commit 7eb7699 into rust-lang:master Jan 14, 2016
@alexcrichton alexcrichton deleted the move-wrapping-and-fill-out branch January 21, 2016 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants