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 details about integer overflow #44

Merged
merged 3 commits into from
Jun 5, 2017

Conversation

mrhota
Copy link
Contributor

@mrhota mrhota commented Apr 18, 2017

CC #9

@mrhota
Copy link
Contributor Author

mrhota commented Apr 18, 2017

I asserted that arithmetic overflow indicates programmer error, which is stronger than the claim in RFC 560. However, I do point users at the wrapping_xxx inherent methods for programmers to annotate and explicitly ask for wrapping arithmetic.

I remained confused after reading the RFC about Rust's overflow semantics. It seemed to indicate two's complement, wrapping overflow is the default if the compiler doesn't insert dynamic checks, but it also indicated an implementation could insert static or dynamic checks at any time to panic or prevent this behavior, excepting of course uses of the wrapping primitives.

@mrhota
Copy link
Contributor Author

mrhota commented Apr 19, 2017

r? @steveklabnik

@mrhota
Copy link
Contributor Author

mrhota commented May 8, 2017

@steveklabnik ping

@steveklabnik
Copy link
Member

Hey @mrhota sorry about this.

I remained confused after reading the RFC about Rust's overflow semantics. It seemed to indicate two's complement, wrapping overflow is the default if the compiler doesn't insert dynamic checks, but it also indicated an implementation could insert static or dynamic checks at any time to panic or prevent this behavior, excepting of course uses of the wrapping primitives.

My understanding is that it's like this:

  1. overflow is a "programmer error", that is, not UB, but not well-formed.
  2. In the case of overflow:
    1. When debug_assertions are enabled, implementations are required to panic at runtime.
    2. When they are not enabled, implementations are allowed to panic or not panic.
    3. If they choose to not panic, overflow is well-defined as two's compliment overflow.
  3. If overflow is a desired behavior, then using the wrapper types is the proper way to do it.

So, it seems like this PR is correct, though maybe not complete, exactly. Thoughts?

@mrhota
Copy link
Contributor Author

mrhota commented May 17, 2017

@steveklabnik that's very helpful. I'll clarify what I have written.

@steveklabnik
Copy link
Member

@mrhota ping!

@mrhota
Copy link
Contributor Author

mrhota commented Jun 5, 2017

@steveklabnik OK, give these changes a shot. I hope it clarifies things

@steveklabnik steveklabnik merged commit 19c5719 into rust-lang:master Jun 5, 2017
@steveklabnik
Copy link
Member

Looks great, thank you!

@mrhota mrhota deleted the int_overflow branch June 5, 2017 15:41
@mrhota mrhota mentioned this pull request Jun 5, 2017
48 tasks
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.

2 participants