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

[regression] inconsistent signed int mod operator between runtime, compiletime, and semfold #12332

Closed
jangko opened this issue Oct 2, 2019 · 1 comment · Fixed by #12402
Closed
Assignees
Labels
Regression Showstopper VM see also `const` label

Comments

@jangko
Copy link
Contributor

jangko commented Oct 2, 2019

With the introduction of Int128 inside Nim compiler, it also introduce this regression.

Example

let ma = -100'i64
let mb = 13
let rtmod = ma mod mb
const ctmod = -100'i64 mod 13
   
debugEcho "SF MOD: ", -100'i64 mod 13 # semfold `mod`
debugEcho "RT MOD: ", rtmod
debugEcho "CT MOD: ", ctmod

Current Output

SF MOD: 9
RT MOD: -9
CT MOD: 9

Expected Output

SF MOD: -9
RT MOD: -9
CT MOD: -9

Possible Solution

The Int128 mod operator assigns wrong sign to the remainder based on divisor sign and not based on dividend sign.

Additional Information

$ nim -v
Nim Compiler Version 1.0.99 [Windows: amd64]
Compiled at 2019-10-02
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: c20778d2d3faa1ad1e9b11a54899b5eef5ac6bca
active boot switches: -d:release
@narimiran
Copy link
Member

The Int128 mod operator assigns wrong sign to the remainder based on divisor sign and not based on dividend sign.

@krux02 ^

@mratsim mratsim added VM see also `const` label Regression labels Oct 3, 2019
krux02 added a commit to krux02/Nim that referenced this issue Oct 10, 2019
Araq pushed a commit that referenced this issue Oct 10, 2019
narimiran pushed a commit to narimiran/Nim that referenced this issue Oct 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Showstopper VM see also `const` label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants