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

Compiler creates invalid out-of-range int8 constants. #13513

Closed
markus-oberhumer opened this issue Feb 26, 2020 · 2 comments · Fixed by #13536
Closed

Compiler creates invalid out-of-range int8 constants. #13513

markus-oberhumer opened this issue Feb 26, 2020 · 2 comments · Fixed by #13536

Comments

@markus-oberhumer
Copy link
Contributor

The compiler creates an invalid int8 constant with the value 128.

Example

const a = high(int8)
const b = a +% 1

doAssert typeof(b) is int8

doAssert b == 128                # INVALID BUT WORKS !!!
doAssert b <= high(int8)         # FAILS

Current Output

Error: unhandled exception: /tmp/test.nim(7, 10) `b <= high(int8)`  [AssertionError]

Expected Output

b should be -128
$ nim -v
Nim Compiler Version 1.1.1 [Linux: amd64]
Compiled at 2020-02-26
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: f091b5a0ee9b5585a08e9130d3ec682a9526ef39
active boot switches: -d:release
@krux02
Copy link
Contributor

krux02 commented Feb 28, 2020

Nice to see you back. I think I implemented all your feedback. I thought we lost you.

I guess this error is my fault. I simply forgot that the +% operator and friends still exists, when I implemented range checks. My personal preference to "fix" this problem would be to deprecate all treat-signed-like-unsigend operators, instead of fixing them. These are the reasons.

  • The percent operators aparently have bugs, probably some unknown bugs as will.
  • Removing the operators also removes their bugs.
  • We don't need those operators, unsigned numbers turned out to the cleaner way to work.
  • Unsigned numbers now finally do work properly at compile time as well as runtime (exception conversion to unsigned causes error in VM; works in RT #12700).
  • Eventually removing the operators thins out system.nim a module that struggles with being bloated.
  • It simplifies the compiler.
  • It simplifies Nim, as new users don't have to learn about these operators anymore.

In your case I would recommend to use this code:

const a = high(int8)
const b = cast[int8](cast[uint8](a) + 1)

doAssert typeof(b) is int8

doAssert b == 128                # FAILS
doAssert b <= high(int8)         # OK

krux02 added a commit to krux02/Nim that referenced this issue Feb 28, 2020
@krux02
Copy link
Contributor

krux02 commented Feb 28, 2020

I just relized that fixing this bug isn't that hard. Still I would really like to deprecate these operators, or at least put them in the unsigned module and out of system.

krux02 added a commit to krux02/Nim that referenced this issue Mar 2, 2020
Araq pushed a commit that referenced this issue Mar 11, 2020
* fixes #13513
* merge tarithmetics in tarithm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants