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

Documentation for abs() on signed integers doesn't mention unsigned_abs() #124152

Closed
Architector4 opened this issue Apr 19, 2024 · 1 comment · Fixed by #124184
Closed

Documentation for abs() on signed integers doesn't mention unsigned_abs() #124152

Architector4 opened this issue Apr 19, 2024 · 1 comment · Fixed by #124184
Assignees
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Architector4
Copy link

Architector4 commented Apr 19, 2024

Location

https://doc.rust-lang.org/std/primitive.i8.html#method.abs
https://doc.rust-lang.org/std/primitive.i16.html#method.abs
https://doc.rust-lang.org/std/primitive.i32.html#method.abs
https://doc.rust-lang.org/std/primitive.i64.html#method.abs
https://doc.rust-lang.org/std/primitive.i128.html#method.abs

Summary

The documentation mentions that the abs method could cause undesired behavior in case a MIN value of the type is passed.

It could use a mention of the unsigned_abs method after this, which can fit all possible return values at cost of converting the value to an unsigned integer instead, which may have been the behavior one wanted when looking for this function in the documentation.

It doesn't help that alphabetically unsigned_abs is sorted all the way down, while abs sits all the way up. A programmer could end up seeing only abs, and assume that they need to either specially handle MIN value, or use the immediately visible abs_diff function below that, with second parameter set to 0 to achieve the wanted effect. (the latter could use a clippy lint? lol)

@Architector4 Architector4 added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Apr 19, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 19, 2024
@Architector4 Architector4 changed the title Documentation for abs() on signed integer doesn't mention unsigned_abs() Documentation for abs() on signed integers doesn't mention unsigned_abs() Apr 19, 2024
@gurry
Copy link
Contributor

gurry commented Apr 20, 2024

@rustbot claim

@bors bors closed this as completed in 24b8c54 Apr 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 21, 2024
Rollup merge of rust-lang#124184 - gurry:124152-suggest-unsigned-abs-in-abs-doc, r=jhpratt

Suggest using `unsigned_abs` in `abs` documentation

Fixes rust-lang#124152
@saethlin saethlin added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants