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. #1678

Closed
wants to merge 3 commits into from

Conversation

jethrogb
Copy link
Contributor

@jethrogb jethrogb commented Jul 14, 2016

@jethrogb jethrogb changed the title wrapping_abs RFC Add a wrapping_abs function to all signed integer types. Jul 14, 2016
@ExpHP
Copy link

ExpHP commented Jul 15, 2016

Can you name a use case? It's hard for me to picture when it would be useful to receive a value of a different type when you want the absolute value of a number.

An alternative could be checked_abs(self) -> Option<Self> (which would use checked_neg for negatives).


Edit: I misread the proposal and thought it was suggesting that e.g. wrapping_abs(self: i32) -> u32. Now that I understand the proposal it feels rather like something that was simply "forgotten" from the standard library!

@jethrogb
Copy link
Contributor Author

jethrogb commented Jul 15, 2016

Obtaining the absolute value is going to always result in a positive number, so why would you not want to store that in an unsigned type? In any case, one example where this is useful for converting a number to a sign-and-magnitude representation:

(signed.is_negative(),signed.wrapping_abs() as u64)

The alternative you mention is not really an alternative but it is something that could be added to this RFC.

@jethrogb
Copy link
Contributor Author

Added checked_abs and overflowing_abs to the RFC. I didn't add saturating_abs since there is no saturating_neg.

@jethrogb jethrogb changed the title Add a wrapping_abs function to all signed integer types. Add non-panicking abs() functions to all signed integer types. Jul 15, 2016
@glaebhoerl
Copy link
Contributor

Rendered link seems to be broken

@glaebhoerl
Copy link
Contributor

glaebhoerl commented Jul 16, 2016

Earlier proposal for reference: Change abs() to return unsigned integers

@jethrogb
Copy link
Contributor Author

Rendered link seems to be broken

Fixed

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jul 17, 2016
This RFC proposes to add the following:

```rust
pub fn checking_abs(self) -> Option<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

checked_abs, not checking, would seem to be more consistent with existing conventions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course.

@brson brson self-assigned this Jul 19, 2016
@brson
Copy link
Contributor

brson commented Jul 19, 2016

Do all these have concrete use cases or are they just here to provide symmetry to other numeric methods? ISTM like checked_abs is the only one that really matters.

@jethrogb
Copy link
Contributor Author

See above for a usecase for wrapping. I don't know of one for overflowing.

@alexcrichton
Copy link
Member

🔔 This RFC is now entering its week-long final comment period 🔔

The libs team is leaning towards merging this as is, and we'd also be willing to just take a PR for this as well if you'd prefer to take that route @jethrogb :)

@alexcrichton alexcrichton added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed I-nominated labels Jul 26, 2016
@jethrogb
Copy link
Contributor Author

rust-lang/rust#35058

@alexcrichton
Copy link
Member

Thanks @jethrogb! In that case I'll just close this in favor of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants