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

clarify extreme operator behaviour #1237

Merged
merged 2 commits into from Sep 18, 2015

Conversation

Projects
None yet
7 participants
@Gankro
Copy link
Contributor

Gankro commented Aug 4, 2015

This is just clarifying things which were agreed on in various places but poorly specified.

rendered draft

enabled this will panic. When checking is disabled this will two's complement
wrap.
- The operations `/`, `%` are nonsensical for the arguments `INT_MIN` and `-1`.
When this occurs there is an unconditional panic.

This comment has been minimized.

@eddyb

eddyb Aug 5, 2015

Member

Maybe division and remainder by 0 are "nonsensical", but INT_MIN / -1 is an overflow, the same way -INT_MIN is (which isn't caught, not even in a debug build).

This comment has been minimized.

@nagisa

nagisa Aug 5, 2015

Contributor

rust-lang/rust#24500 and rust-lang/rust#23154 and rust-lang/rust#22020 all seem to involve unary negation checking for overflow, but in practice this seems to be not working indeed in the context of const-eval.

This comment has been minimized.

@eddyb

eddyb Aug 5, 2015

Member

-std::i32::MIN does give a const-eval error, but let m = std::i32::MIN; -m doesn't (so there's no runtime checking).

This comment has been minimized.

@nagisa

nagisa Aug 5, 2015

Contributor

Seems to work for me.

This comment has been minimized.

@eddyb

eddyb Aug 5, 2015

Member

Oops, I used playbot - does it run in release mode?

[17:13] <eddyb> playbot: let x = std::i32::MIN; -x
[17:13] [Notice] -playbot to #rust-internals- -2147483648

This comment has been minimized.

@nikomatsakis

nikomatsakis Aug 7, 2015

Contributor

Regarding -INT_MIN and INT_MIN/-1 -- for whatever reason, division had code to check and panic, but multiplication did two's complement wrapping. We have preserved that behavior, afaik, inconsistent or not, and hence these edits correctly describe the current situation.

- Shift operations (`<<`, `>>`) can shift a value of width `N` by more
than `N` bits.
than `N` bits. This is prevented by unconditionally masking the bits

This comment has been minimized.

@eddyb

eddyb Aug 5, 2015

Member

What is "this", that is prevented here?

This comment has been minimized.

@nikomatsakis

nikomatsakis Aug 7, 2015

Contributor

@eddyb

What is "this", that is prevented here?

The undefined behavior that results from an overlong shift, presumably.

This comment has been minimized.

@eddyb

eddyb Aug 7, 2015

Member

Yes, that's what I assume it means, but there's no description of that undefined behavior which "this" could refer to.

This comment has been minimized.

@Gankro

Gankro Aug 7, 2015

Author Contributor

Strictly speaking at this level Undefined Behaviour in LLVM in not an object -- this is Rust's semantics. Will clarify.

overflow.
- The operations `+`, `-`, `*`, can underflow and overflow. When checking is
enabled this will panic. When checking is disabled this will two's complement
wrap.

This comment has been minimized.

@nagisa

nagisa Aug 5, 2015

Contributor

Do we guarantee this for platforms where arithmetic is not two’s complement?

This comment has been minimized.

@eddyb

eddyb Aug 5, 2015

Member

I don't think LLVM supports anything other than two's complement.

This comment has been minimized.

@Gankro

Gankro Aug 5, 2015

Author Contributor

Yeah. We also don't acknowledged floating point rounding modes for similar reasons.

This comment has been minimized.

@oli-obk

oli-obk Aug 6, 2015

Contributor

actually this is currently only the behavior when the operation cannot be constant evaluated. Even with checking disabled, const evaluatable cases will cause a compiler error. see #1229 for discussion

@nrc nrc added the T-lang label Aug 6, 2015

@nikomatsakis nikomatsakis self-assigned this Aug 6, 2015

@Gankro

This comment has been minimized.

Copy link
Contributor Author

Gankro commented Aug 7, 2015

Clarified.

@nikomatsakis

This comment has been minimized.

Copy link

nikomatsakis commented on text/0560-integer-overflow.md in 32ed8d4 Aug 7, 2015

seems fine, but it may be worth nothing that this is the behavior that the X86 does (and Java, as well, not coincidentally I think)

This comment has been minimized.

Copy link

nikomatsakis replied Aug 7, 2015

(or at least a logical extension of it)

@nikomatsakis

This comment has been minimized.

Copy link

nikomatsakis commented on text/0560-integer-overflow.md in 32ed8d4 Aug 7, 2015

Similarly, the reason here is that LLVM's sdiv instruction makes those two cases undefined behavior: "Division by zero leads to undefined behavior. Overflow also leads to undefined behavior; this is a rare case, but can occur, for example, by doing a 32-bit division of -2147483648 by -1." I don't believe there is a version of div where they are defined behavior. We could, of course, opt to do the "overflow" for -1 ourselves. Division by zero OTOH has traditionally been a fault in many CPUs and programming languages.

@glaebhoerl

This comment has been minimized.

Copy link
Contributor

glaebhoerl commented Aug 8, 2015

As @bill-myers originally pointed out here INT_MIN % -1 is mathematically well-defined to be 0.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Sep 4, 2015

Hear ye, hear ye. This RFC is entering final comment period.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Sep 4, 2015

@glaebhoerl true. I guess we could consider altering the behavior, but I think this RFC is basically aiming at documenting existing behavior, and that particular result is long-standing.

@glaebhoerl

This comment has been minimized.

Copy link
Contributor

glaebhoerl commented Sep 5, 2015

@nikomatsakis Then shall I open a separate issue to keep track of that possibility?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Sep 9, 2015

On Sat, Sep 05, 2015 at 01:53:14AM -0700, Gábor Lehel wrote:

@nikomatsakis Then shall I open a separate issue to keep track of that possibility?

Sure.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Sep 18, 2015

Huzzah! The language design team has decided to accept this RFC.

nikomatsakis added a commit that referenced this pull request Sep 18, 2015

Merge pull request #1237 from Gankro/clarify-math
clarify extreme operator behaviour

@nikomatsakis nikomatsakis merged commit dd79587 into rust-lang:master Sep 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.