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

conversion to uint64 is bounded to int64 values (when passing as a literal) #1085

Closed
thehowl opened this issue Aug 30, 2023 · 0 comments · Fixed by #1423
Closed

conversion to uint64 is bounded to int64 values (when passing as a literal) #1085

thehowl opened this issue Aug 30, 2023 · 0 comments · Fixed by #1423
Assignees
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@thehowl
Copy link
Member

thehowl commented Aug 30, 2023

all of the following fail:

// 9223372036854775808 == 2**63 (first value not supported by int64)
var x = uint64(9223372036854775808)
var x uint64 = uint64(9223372036854775808)

const x = uint64(9223372036854775808)
const x uint64 = uint64(9223372036854775808)
$ gno run a.go
--- preprocess stack ---
stack 0: file{ package main; import fmt fmt; var x<VPBlock(2,1)> = (const-type uint64)((const (9223372036854775808 <untyped> bigint))); func main() { fmt<VPUverse(0)>.Pr
intln(x<VPUverse(0)>) } }
------------------------
panic: bigint overflows target kind [recovered]
        panic: main/a.go:5: bigint overflows target kind [recovered]
        panic: main/a.go:5: bigint overflows target kind

goroutine 1 [running]:
github.com/gnolang/gno/gnovm/pkg/gnolang.predefineNow.func1()
        /home/howl/oc/gno/gnovm/pkg/gnolang/preprocess.go:2851 +0x19c
panic({0xc89140, 0xc000316e80})
        /usr/lib/go/src/runtime/panic.go:884 +0x213
github.com/gnolang/gno/gnovm/pkg/gnolang.Preprocess.func2.1()
        /home/howl/oc/gno/gnovm/pkg/gnolang/preprocess.go:173 +0x46d
panic({0xbf0b00, 0xf4c640})
        /usr/lib/go/src/runtime/panic.go:890 +0x263
github.com/gnolang/gno/gnovm/pkg/gnolang.ConvertUntypedBigintTo(0xc0002ff770, {0xc000136018?}, {0xf58868, 0xf4b830})
        /home/howl/oc/gno/gnovm/pkg/gnolang/values_conversions.go:1070 +0x7a5
github.com/gnolang/gno/gnovm/pkg/gnolang.ConvertUntypedTo(0xc0002ff770, {0x0, 0x0})
        /home/howl/oc/gno/gnovm/pkg/gnolang/values_conversions.go:941 +0x8ff
github.com/gnolang/gno/gnovm/pkg/gnolang.convertConst({0xf651b0, 0xc000188240}, {0xf62cd8?, 0xc0002b38c0?}, 0xc0002ff740, {0x0?, 0x0?})
        /home/howl/oc/gno/gnovm/pkg/gnolang/preprocess.go:2355 +0x2b6
...

The only way to use a large literal like that is the following:

var x uint64 = 9223372036854775808
const x uint64 = 9223372036854775808

// values less than 2**63 are regularly supported
var x = uint64(9223372036854775807)

(Part of a series of issues encountered while developing gnochess)

@thehowl thehowl added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related labels Aug 30, 2023
@moul moul added this to the 🚀 main.gno.land milestone Sep 6, 2023
@deelawn deelawn self-assigned this Dec 4, 2023
@deelawn deelawn linked a pull request Dec 8, 2023 that will close this issue
7 tasks
deelawn added a commit that referenced this issue Jan 11, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->
Addresses #1085 

The call to `convConst` should include the type in this case. This
happens while processing a `CallExpr` and in this case the expression is
a function call to the built function `uint64`. We have the expected
type, so use when converting the constant to a typed value. If this type
is still nil, then it will behave as it does currently.

However, the concrete type should only be supplied to `convertConst` if
the call expression's type and the constant expression's type both have
the same underlying _unsigned type_ -- the base type of any value of
these types were they to be evaluated as literals before being assigned
a type.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
gfanton pushed a commit to moul/gno that referenced this issue Jan 18, 2024
<!-- please provide a detailed description of the changes made in this
pull request. -->
Addresses gnolang#1085 

The call to `convConst` should include the type in this case. This
happens while processing a `CallExpr` and in this case the expression is
a function call to the built function `uint64`. We have the expected
type, so use when converting the constant to a typed value. If this type
is still nil, then it will behave as it does currently.

However, the concrete type should only be supplied to `convertConst` if
the call expression's type and the constant expression's type both have
the same underlying _unsigned type_ -- the base type of any value of
these types were they to be evaluated as literals before being assigned
a type.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: 🚀 Needed for Launch
Development

Successfully merging a pull request may close this issue.

3 participants