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.

Sign up for GitHub

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

Jump to bottom

clarify extreme operator behaviour #1237

Merged
merged 2 commits into from
Sep 18, 2015
Merged

clarify extreme operator behaviour #1237

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8637d7a
clarify extreme operator behaviour
Gankra Aug 4, 2015
32ed8d4
clarify reasoning
Gankra Aug 7, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions text/0560-integer-overflow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,14 @@ The error conditions that can arise, and their defined results, are as
follows. The intention is that the defined results are the same as the
defined results today. The only change is that now a panic may result.

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

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

- The operations `/`, `%` are nonsensical for the arguments `INT_MIN` and `-1`.
When this occurs there is an unconditional panic.
Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

Seems to work for me.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Member

Choose a reason for hiding this comment

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

What is "this", that is prevented here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@eddyb

What is "this", that is prevented here?

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

of the right-hand-side to wrap modulo `N`.

## Enabling overflow checking

Expand All @@ -145,7 +149,7 @@ potential overflow (and, in particular, for code where overflow is
expected and normal, they will be immediately guided to use the
wrapping methods introduced below). However, because these checks will
be compiled out whenever an optimized build is produced, final code
wilil not pay a performance penalty.
will not pay a performance penalty.

In the future, we may add additional means to control when overflow is
checked, such as scoped attributes or a global, independent
Expand Down Expand Up @@ -451,17 +455,7 @@ were:

# Unresolved questions

The C semantics of wrapping operations in some cases are undefined:

- `INT_MIN / -1`, `INT_MIN % -1`
- Shifts by an excessive number of bits

This RFC takes no position on the correct semantics of these
operations, simply preserving the existing semantics. However, it may
be worth trying to define the wrapping semantics of these operations
in a portable way, even if that implies some runtime cost. Since these
are all error conditions, this is an orthogonal topic to the matter of
overflow.
None today (see Updates section below).

# Future work

Expand Down Expand Up @@ -491,6 +485,10 @@ Since it was accepted, the RFC has been updated as follows:
2. `as` was changed to restore the behavior before the RFC (that is,
it truncates to the target bitwidth and reinterprets the highest
order bit, a.k.a. sign-bit, as necessary, as a C cast would).
3. Shifts were specified to mask off the bits of over-long shifts.
4. Overflow was specified to be two's complement wrapping (this was mostly
a clarification).
5. `INT_MIN / -1` and `INT_MIN % -1` panics.

# Acknowledgements and further reading

Expand Down