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

feat(gnovm): align Gno constant handling with Go specifications #2828

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3d5030a
feat(gnovm): align Gno constant handling with Go specifications
omarsy Sep 21, 2024
2543294
Merge branch 'master' into fix/2628
omarsy Oct 18, 2024
bb2d9fd
feat: correct test
omarsy Oct 18, 2024
c7643eb
Merge branch 'master' into fix/2628
omarsy Oct 22, 2024
b80a890
feat: add a todo for native type conversion
omarsy Oct 23, 2024
ff74523
feat: uncomment package branch
omarsy Oct 23, 2024
761ef1a
feat: use println
omarsy Oct 23, 2024
5fa41e9
feat: add test
omarsy Oct 23, 2024
90c8c5b
fix: use evalStaticType only for type
omarsy Oct 23, 2024
e74e6e9
feat: add more test
omarsy Oct 23, 2024
3a52e14
feat: add missing init expr
omarsy Oct 24, 2024
f53a19f
feat: add real and imag on todo
omarsy Nov 7, 2024
194883a
Merge remote-tracking branch 'upstream/master' into fix/2628
omarsy Nov 26, 2024
13136c5
fix: test
omarsy Nov 26, 2024
27e69b4
feat: correct merge
omarsy Nov 27, 2024
5dc615a
feat: add test
omarsy Nov 27, 2024
6fb3cf7
Merge branch 'master' into fix/2628
omarsy Nov 27, 2024
743962c
fix: test
omarsy Nov 27, 2024
35d6ae1
Merge branch 'master' into fix/2628
omarsy Dec 3, 2024
459f0da
Merge branch 'master' into fix/2628
omarsy Dec 7, 2024
1ba4872
refactor: replace checkConstantExpr with assertValidConstExpr for imp…
omarsy Dec 7, 2024
54878f0
refactor: rename assertValidConstantExprRecursively to assertValidCon…
omarsy Dec 9, 2024
72b8910
Merge branch 'master' into fix/2628
omarsy Dec 9, 2024
88dae32
Merge branch 'master' into fix/2628
omarsy Dec 10, 2024
59a7d85
Merge branch 'master' into fix/2628
omarsy Dec 11, 2024
9001059
fix(gnovm): check type
omarsy Dec 26, 2024
c45f63e
fix(gnovm): enhance constant evaluation for array and map types
omarsy Dec 26, 2024
7083830
refactore(gnovm): some refactor
omarsy Dec 26, 2024
4da7934
fix(gnovm): remove unnecessary panic for non-constant array type expr…
omarsy Dec 26, 2024
6ef45d4
Merge branch 'master' into fix/2628
omarsy Dec 27, 2024
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
2 changes: 1 addition & 1 deletion examples/gno.land/r/demo/keystore/keystore_test.gno
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func TestRender(t *testing.T) {
const (
var (
author1 std.Address = testutils.TestAddress("author1")
author2 std.Address = testutils.TestAddress("author2")
)
Expand Down
2 changes: 1 addition & 1 deletion examples/gno.land/r/demo/microblog/microblog_test.gno
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

func TestMicroblog(t *testing.T) {
const (
var (
author1 std.Address = testutils.TestAddress("author1")
author2 std.Address = testutils.TestAddress("author2")
)
Expand Down
1 change: 1 addition & 0 deletions gnovm/pkg/gnolang/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -2294,6 +2294,7 @@ func preprocess1(store Store, ctx BlockNode, n Node) Node {
// NOTE: may or may not be a *ConstExpr,
// but if not, make one now.
for i, vx := range n.Values {
assertValidConstExpr(store, last, n, vx)
n.Values[i] = evalConst(store, last, vx)
}
} else {
Expand Down
174 changes: 174 additions & 0 deletions gnovm/pkg/gnolang/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,180 @@
}
}

func assertValidConstExpr(store Store, last BlockNode, n *ValueDecl, expr Expr) {
if n.Type != nil {
nt := evalStaticType(store, last, n.Type)
if xnt, ok := nt.(*NativeType); ok {
nt = go2GnoBaseType(xnt.Type)
}

Check warning on line 223 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L222-L223

Added lines #L222 - L223 were not covered by tests

if _, ok := baseOf(nt).(PrimitiveType); !ok {
panic(fmt.Sprintf("invalid constant type %s", nt.String()))
}
}

nt := evalStaticTypeOf(store, last, expr)
if xnt, ok := nt.(*NativeType); ok {
nt = go2GnoBaseType(xnt.Type)
}

Check warning on line 233 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L232-L233

Added lines #L232 - L233 were not covered by tests

if nt == nil {
panic(fmt.Sprintf("%s (variable of type nil) is not constant", expr))

Check warning on line 236 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L236

Added line #L236 was not covered by tests
}

if _, ok := baseOf(nt).(PrimitiveType); !ok {
panic(fmt.Sprintf("%s (variable of type %s) is not constant", expr, nt))
}

assertValidConstValue(store, last, expr, nil)
}

func assertValidConstValue(store Store, last BlockNode, currExpr, parentExpr Expr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

At the same time, I’m wondering—many of the checks here are for functions like len and cap. So, is this recursive call really necessary? Could most of the cases be handled under callExpr instead? Please consider this.

Apologies for the continuous iterations in reviewing this PR. Thank you for understanding.

Main:
switch currExpr := currExpr.(type) {
case *NameExpr:
t := evalStaticTypeOf(store, last, currExpr)
if _, ok := t.(*TypeType); ok {
t = evalStaticType(store, last, currExpr)
}
// special case for len, cap
if isParentCallExprWithArrayArg(t, parentExpr) {
break Main
ltzmaxwell marked this conversation as resolved.
Show resolved Hide resolved
}
panic(fmt.Sprintf("%s (variable of type %s) is not constant", currExpr.Name, t))
case *TypeAssertExpr:
ty := evalStaticTypeOf(store, last, currExpr)
if _, ok := ty.(*TypeType); ok {
ty = evalStaticType(store, last, currExpr)
}

Check warning on line 263 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L262-L263

Added lines #L262 - L263 were not covered by tests
// special case for len, cap
if isParentCallExprWithArrayArg(ty, parentExpr) {
break Main

Check warning on line 266 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L266

Added line #L266 was not covered by tests
}
panic(fmt.Sprintf("%s (comma, ok expression of type %s) is not constant", currExpr.String(), currExpr.Type))
case *IndexExpr:
ty := evalStaticTypeOf(store, last, currExpr)
if _, ok := ty.(*TypeType); ok {
ty = evalStaticType(store, last, currExpr)
}

Check warning on line 273 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L272-L273

Added lines #L272 - L273 were not covered by tests
// TODO: should add a test after the fix of https://github.com/gnolang/gno/issues/3409
// special case for len, cap
if isParentCallExprWithArrayArg(ty, parentExpr) {
break Main
}

panic(fmt.Sprintf("%s (variable of type %s) is not constant", currExpr.String(), currExpr.X))
case *CallExpr:
ift := evalStaticTypeOf(store, last, currExpr.Func)
switch baseOf(ift).(type) {
case *FuncType:
tup := evalStaticTypeOfRaw(store, last, currExpr).(*tupleType)

// check for built-in functions
petar-dambovaliev marked this conversation as resolved.
Show resolved Hide resolved
if cx, ok := currExpr.Func.(*ConstExpr); ok {
if fv, ok := cx.V.(*FuncValue); ok {
if fv.PkgPath == uversePkgPath {
// TODO: should support min, max, real, imag
switch {
case fv.Name == "len":
assertValidConstValue(store, last, currExpr.Args[0], currExpr)
break Main
case fv.Name == "cap":
assertValidConstValue(store, last, currExpr.Args[0], currExpr)
break Main
}
}
}
}

switch {
case len(tup.Elts) == 0:
panic(fmt.Sprintf("%s (no value) used as value", currExpr.String()))

Check warning on line 306 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L305-L306

Added lines #L305 - L306 were not covered by tests
case len(tup.Elts) == 1:
panic(fmt.Sprintf("%s (value of type %s) is not constant", currExpr.String(), tup.Elts[0]))
default:
panic(fmt.Sprintf("multiple-value %s (value of type %s) in single-value context", currExpr.String(), tup.Elts))

Check warning on line 310 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L309-L310

Added lines #L309 - L310 were not covered by tests
}
case *TypeType:
for _, arg := range currExpr.Args {
assertValidConstValue(store, last, arg, currExpr)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

please check this:

package main

const a = interface{}(nil)

func main() {
	println("ok")
}

seems also need to check the type, right?

Copy link
Member Author

@omarsy omarsy Dec 26, 2024

Choose a reason for hiding this comment

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

My bad break on my last refactoring when I move the check type

Nice catch ^^

Copy link
Contributor

Choose a reason for hiding this comment

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

so the above clause: case *TypeType is not needed? didn't see any test case targeting this.

case *NativeType:
ltzmaxwell marked this conversation as resolved.
Show resolved Hide resolved
// Todo: should add a test after the fix of https://github.com/gnolang/gno/issues/3006
ty := evalStaticType(store, last, currExpr.Func)
panic(fmt.Sprintf("%s (variable of type %s) is not constant", currExpr.String(), ty))
default:
panic(fmt.Sprintf(
"unexpected func type %v (%v)",
ift, reflect.TypeOf(ift)))

Check warning on line 323 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L316-L323

Added lines #L316 - L323 were not covered by tests
}
case *BinaryExpr:
assertValidConstValue(store, last, currExpr.Left, parentExpr)
assertValidConstValue(store, last, currExpr.Right, parentExpr)
case *SelectorExpr:
xt := evalStaticTypeOf(store, last, currExpr.X)
switch xt := xt.(type) {
case *PackageType:
var pv *PackageValue
if cx, ok := currExpr.X.(*ConstExpr); ok {
// NOTE: *Machine.TestMemPackage() needs this
// to pass in an imported package as *ConstEzpr.
pv = cx.V.(*PackageValue)

Check warning on line 336 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L334-L336

Added lines #L334 - L336 were not covered by tests
} else {
// otherwise, packages can only be referred to by
// *NameExprs, and cannot be copied.
pvc := evalConst(store, last, currExpr.X)
pv_, ok := pvc.V.(*PackageValue)
if !ok {
panic(fmt.Sprintf(
"missing package in selector expr %s",
currExpr.String()))

Check warning on line 345 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L343-L345

Added lines #L343 - L345 were not covered by tests
}
pv = pv_
}
if pv.GetBlock(store).Source.GetIsConst(store, currExpr.Sel) {
break Main
}

tt := pv.GetBlock(store).Source.GetStaticTypeOf(store, currExpr.Sel)
panic(fmt.Sprintf("%s (variable of type %s) is not constant", currExpr.String(), tt))

Check warning on line 354 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L353-L354

Added lines #L353 - L354 were not covered by tests
case *PointerType, *DeclaredType, *StructType, *InterfaceType, *TypeType, *NativeType:
Copy link
Contributor

@ltzmaxwell ltzmaxwell Dec 31, 2024

Choose a reason for hiding this comment

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

seems *TypeType does not apply. please double check.

ty := evalStaticTypeOf(store, last, currExpr)
if _, ok := ty.(*TypeType); ok {
ty = evalStaticType(store, last, currExpr)
}

Check warning on line 359 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L358-L359

Added lines #L358 - L359 were not covered by tests

// special case for len, cap
if isParentCallExprWithArrayArg(ty, parentExpr) {
break Main
}
panic(fmt.Sprintf("%s (variable of type %s) is not constant", currExpr.String(), ty))
default:
panic(fmt.Sprintf(
"unexpected selector expression type %v",
reflect.TypeOf(xt)))

Check warning on line 369 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L366-L369

Added lines #L366 - L369 were not covered by tests
}

case *ConstExpr:
case *BasicLitExpr:

Check warning on line 373 in gnovm/pkg/gnolang/type_check.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L373

Added line #L373 was not covered by tests
case *CompositeLitExpr:
Copy link
Contributor

@ltzmaxwell ltzmaxwell Dec 26, 2024

Choose a reason for hiding this comment

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

can we just check parent and panic here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we verify the expression type before calling this function, this case should not occur. Removed for array: 4da7934

Copy link
Contributor

Choose a reason for hiding this comment

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

will it be more straightforward that just check if parentExpr exist, and the type of compositeLitExpr is array type, otherwise panic?

assertValidConstValue(store, last, currExpr.Type, parentExpr)
default:
ift := evalStaticTypeOf(store, last, currExpr)
if _, ok := ift.(*TypeType); ok {
ift = evalStaticType(store, last, currExpr)
}
panic(fmt.Sprintf("%s (variable of type %s) is not constant", currExpr.String(), ift))
}
}

func isParentCallExprWithArrayArg(currType Type, parentExpr Expr) bool {
_, okArray := baseOf(currType).(*ArrayType)
_, okCallExpr := parentExpr.(*CallExpr)
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment.


return okArray && okCallExpr
}

// checkValDefineMismatch checks for mismatch between the number of variables and values in a ValueDecl or AssignStmt.
func checkValDefineMismatch(n Node) {
var (
Expand Down
11 changes: 11 additions & 0 deletions gnovm/tests/files/const23.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package main

import "fmt"

func main() {
const t []string = []string{}
fmt.Println(t)
}

// Error:
// main/files/const23.gno:6:8: invalid constant type []string
79 changes: 79 additions & 0 deletions gnovm/tests/files/const24.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package main

import (
"fmt"
"time"
)

func main() {
const a int = 1_000_000
const b byte = byte(1)
const c float64 = 1_000_000.000
const d string = "Hello, World!"
const e rune = 'a'
const g bool = true
const h uint = 1_000
const i int8 = 1
const j int16 = 1
const k int32 = 1
const l int64 = 1
const m uint8 = 1
const n uint16 = 1
const o uint32 = 1
const p uint64 = 1
const r float32 = 1_000_000.000
const s = r
const t = len("s")
const u = 1 + len("s") + 3
ars := [10]string{}
const v = len(ars)
const w = cap(ars)
const x = time.Second

fmt.Println(a)
fmt.Println(b)
fmt.Println(c)
fmt.Println(d)
fmt.Println(e)
fmt.Println(g)
fmt.Println(h)
fmt.Println(i)
fmt.Println(j)
fmt.Println(k)
fmt.Println(l)
fmt.Println(m)
fmt.Println(n)
fmt.Println(o)
fmt.Println(p)
fmt.Println(r)
fmt.Println(s)
fmt.Println(t)
fmt.Println(u)
fmt.Println(v)
fmt.Println(w)
println(x)
}

// Output:
// 1000000
// 1
// 1e+06
// Hello, World!
// 97
// true
// 1000
// 1
// 1
// 1
// 1
// 1
// 1
// 1
// 1
// 1e+06
// 1e+06
// 1
// 5
// 10
// 10
// 1s
11 changes: 11 additions & 0 deletions gnovm/tests/files/const25.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package main

import "fmt"

func main() {
const t = []string{"1"}
fmt.Println(t)
}

// Error:
// main/files/const25.gno:6:8: [](const-type string){(const ("1" string))} (variable of type []string) is not constant
15 changes: 15 additions & 0 deletions gnovm/tests/files/const26.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import "fmt"

func v() string {
return ""
}

func main() {
const t = v()
fmt.Println(t)
}

// Error:
// main/files/const26.gno:10:8: v<VPBlock(3,0)>() (value of type string) is not constant
16 changes: 16 additions & 0 deletions gnovm/tests/files/const27.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package main

import "fmt"

func v() string {
return ""
}

func main() {
var i interface{} = 1
const t, ok = i.(int)
fmt.Println(t, ok)
}

// Error:
// main/files/const27.gno:11:8: i<VPBlock(1,0)>.((const-type int)) (comma, ok expression of type (const-type int)) is not constant
12 changes: 12 additions & 0 deletions gnovm/tests/files/const28.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package main

import "fmt"

func main() {
var s []string = []string{"1"}
const t, ok = s[0]
fmt.Println(t, ok)
}

// Error:
// main/files/const28.gno:7:8: s<VPBlock(1,0)>[(const (0 int))] (variable of type s<VPBlock(1,0)>) is not constant
12 changes: 12 additions & 0 deletions gnovm/tests/files/const29.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package main

import "fmt"

func main() {
s := "1"
const t = s
fmt.Println(t)
}

// Error:
// main/files/const29.gno:7:8: s (variable of type string) is not constant
15 changes: 15 additions & 0 deletions gnovm/tests/files/const30.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import "fmt"

func v() {
return
}

func main() {
const t = v()
fmt.Println(t)
}

// Error:
// main/files/const30.gno:10:8: v<VPBlock(3,0)> (no value) used as value
Loading
Loading