-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Generic int64
match does not generate correct literal on JS
#24233
Comments
metagn
added a commit
to metagn/Nim
that referenced
this issue
Oct 4, 2024
Araq
pushed a commit
that referenced
this issue
Oct 7, 2024
fixes #24233 Integer literals with type `int` can match `int64` with a generic match. Normally this would generate an conversion via `isFromIntLit`, but when it matches with a generic match (`isGeneric`) the node is left alone and continues to have type `int` (related to #4858, but separate; since `isFromIntLit > isGeneric` it doesn't propagate). This did not cause problems on the C backend up to this point because either the compiler generated a cast when generating the C code or it was implicitly casted in the C code itself. On the JS backend however, we need to generate `int64` and `int` values differently, so we copy the integer literal and give it the matched type now instead. This is somewhat risky even if CI passes but it's required to make the times module work without [this workaround](https://github.com/nim-lang/Nim/blob/7dfadb8b4e95d09981fbeb01d85b12f23946c3e7/lib/pure/times.nim#L219-L238) on `--jsbigint64:on` (the default). CI exposed an issue: When matching an int literal to a generic parameter in a generic instantiation, the literal is only treated like a value if it has `int literal` type, but if it has the type `int`, it gets transformed into literally the type `int` (#12664, #13906), which breaks the tests t14193 and t12938. To deal with this, we don't give it the type `int` if we are in a generic instantiation and preserve the `int literal` type.
Araq
pushed a commit
that referenced
this issue
Oct 19, 2024
refs #6978, refs #6752, refs #21613, refs #24234 The `jsNoInt64`, `whenHasBigInt64`, `whenJsNoBigInt64` templates are replaced with bool constants to use with `when`. Weird that I didn't do this in the first place. The `whenJsNoBigInt64` template was also slightly misleading. The first branch was compiled for both no bigint64 on JS as well as on C/C++. It seems only `trandom` depended on this by mistake. The workaround for #6752 added in #6978 to `times` is also removed with `--jsbigint64:on`, but #24233 was also encountered with this, so this PR depends on #24234.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Found when trying to remove these workarounds from times
bar
is passed as a regular integer123
tofoo
, when it should be123n
with--jsbigint64:on
Nim Version
presumably anything after #21613
Current Output
Expected Output
Known Workarounds
No response
Additional Information
No response
The text was updated successfully, but these errors were encountered: