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

Fix checked operations documentation. #332

Merged
merged 2 commits into from
Aug 14, 2024
Merged

Conversation

ocstl
Copy link
Contributor

@ocstl ocstl commented Jul 26, 2024

Closes #13 . Remove the implication that wrapping is the default behaviour.

The documentation mentions "underflow" (for example, in CheckedSub), which I left as is. But Rust's documentation tends to refer to "overflow" exclusively (see i32::checked_sub).

I've recently been made aware that overflow is probably the correct term in both cases for integers (positive and negative), while underflow is strictly reserved for floating points. Either way, it's not a big deal and I'm not bothered by it, as the meaning is clear, but I thought I should mention it.

Remove the implication that wrapping is the default behaviour.
@cuviper
Copy link
Member

cuviper commented Jul 26, 2024

Let's change "underflow" -- the reference calls it "overflow" for results less than the minimum:
https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow

... and "underflow" isn't found in the reference at all!

- Remove the implication that wrapping is the default behaviour for `CheckedEuclid` methods.
- Replace mentions of 'underflow' with 'overflow'.
@ocstl
Copy link
Contributor Author

ocstl commented Jul 27, 2024

I guess the reference settles that!

I also missed the CheckedEuclid methods in the first commit, but they are now fixed as well.

@cuviper
Copy link
Member

cuviper commented Aug 14, 2024

Thanks!

@cuviper cuviper added this pull request to the merge queue Aug 14, 2024
Merged via the queue into rust-num:master with commit 022f250 Aug 14, 2024
4 checks passed
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.

Documentation for checked operations says "instead of wrapping around on underflow"
2 participants