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

Add non-panicking abs() functions to all signed integer types. #35058

Merged
merged 1 commit into from
Jul 30, 2016

Conversation

jethrogb
Copy link
Contributor

Currently, calling abs() on one of the signed integer types might panic (in
debug mode at least) because the absolute value of the largest negative value
can not be represented in that signed type. Unlike all other integer
operations, there is currently not a non-panicking version on this function.
This seems to just be an oversight in the design, therefore just adding it now.

@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 @alexcrichton (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.

/// ```
#[unstable(feature = "no_panic_abs", issue = "35057")]
#[inline]
pub fn overflowing_abs(self) -> (Self,bool) {
Copy link
Member

Choose a reason for hiding this comment

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

There should be a space after the ,.

@alexcrichton
Copy link
Member

@bors: r+ 31a0514a2525e31b044969df6e93c41f54e6b025

Thanks @jethrogb!

if self.is_negative() {
self.overflowing_neg()
} else {
self
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be (self, false).

@alexcrichton
Copy link
Member

@bors: r+ bdd6a11f0e636daff50075360180989144879fff

@sanxiyn
Copy link
Member

sanxiyn commented Jul 28, 2016

@bors r-

Documentation test failed in rollup. You need to add #![feature]. See fn step_by in src/core/iter/range.rs for how.

@Manishearth
Copy link
Member

thanks, @sanxiyn

bors added a commit that referenced this pull request Jul 28, 2016
Rollup of 7 pull requests

- Successful merges: #34951, #34963, #34969, #35013, #35037, #35040, #35058
- Failed merges:
Currently, calling abs() on one of the signed integer types might panic (in
debug mode at least) because the absolute value of the largest negative value
can not be represented in that signed type. Unlike all other integer
operations, there is currently not a non-panicking version on this function.
This seems to just be an oversight in the design, therefore just adding it now.
@jethrogb
Copy link
Contributor Author

Fixed and rebased

@alexcrichton
Copy link
Member

@bors: r+ cdc6afe

@alexcrichton
Copy link
Member

@bors: rollup

assuming this isn't very platform-specific and green travis means it's good to go whenever

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 30, 2016
Add non-panicking abs() functions to all signed integer types.

Currently, calling abs() on one of the signed integer types might panic (in
debug mode at least) because the absolute value of the largest negative value
can not be represented in that signed type. Unlike all other integer
operations, there is currently not a non-panicking version on this function.
This seems to just be an oversight in the design, therefore just adding it now.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 30, 2016
Add non-panicking abs() functions to all signed integer types.

Currently, calling abs() on one of the signed integer types might panic (in
debug mode at least) because the absolute value of the largest negative value
can not be represented in that signed type. Unlike all other integer
operations, there is currently not a non-panicking version on this function.
This seems to just be an oversight in the design, therefore just adding it now.
bors added a commit that referenced this pull request Jul 30, 2016
Rollup of 8 pull requests

- Successful merges: #35049, #35058, #35063, #35080, #35090, #35094, #35104, #35106
- Failed merges:
@bors bors merged commit cdc6afe into rust-lang:master Jul 30, 2016
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.

8 participants