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 11 commits into
base: master
Choose a base branch
from
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
18 changes: 18 additions & 0 deletions gnovm/pkg/gnolang/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -2264,6 +2264,7 @@
// NOTE: may or may not be a *ConstExpr,
// but if not, make one now.
for i, vx := range n.Values {
checkConstantExpr(store, last, vx)
n.Values[i] = evalConst(store, last, vx)
}
} else {
Expand Down Expand Up @@ -2344,6 +2345,16 @@
if n.Type != nil {
// only a single type can be specified.
nt := evalStaticType(store, last, n.Type)
if n.Const {
if xnt, ok := nt.(*NativeType); ok {
nt = go2GnoBaseType(xnt.Type)
}

Check warning on line 2351 in gnovm/pkg/gnolang/preprocess.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/preprocess.go#L2350-L2351

Added lines #L2350 - L2351 were not covered by tests

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

Check warning on line 2354 in gnovm/pkg/gnolang/preprocess.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/preprocess.go#L2354

Added line #L2354 was not covered by tests
}
}

for i := 0; i < numNames; i++ {
sts[i] = nt
}
Expand All @@ -2355,6 +2366,13 @@
// derive static type from values.
for i, vx := range n.Values {
vt := evalStaticTypeOf(store, last, vx)
if xnt, ok := vt.(*NativeType); ok {
vt = go2GnoBaseType(xnt.Type)
}

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

Check warning on line 2374 in gnovm/pkg/gnolang/preprocess.go

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/preprocess.go#L2374

Added line #L2374 was not covered by tests
}
sts[i] = vt
}
} else { // T is nil, n not const
Expand Down
109 changes: 109 additions & 0 deletions gnovm/pkg/gnolang/type_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,115 @@
}
}

func checkConstantExpr(store Store, last BlockNode, vx Expr) {
Main:
switch vx := vx.(type) {
case *NameExpr:
t := evalStaticTypeOf(store, last, vx)
if _, ok := t.(*ArrayType); ok {
break Main
}
panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.Name, t))
case *TypeAssertExpr:
panic(fmt.Sprintf("%s (comma, ok expression of type %s) is not constant", vx.String(), vx.Type))
case *IndexExpr:
panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.String(), vx.X))
case *CallExpr:
ift := evalStaticTypeOf(store, last, vx.Func)
switch baseOf(ift).(type) {
case *FuncType:
tup := evalStaticTypeOfRaw(store, last, vx).(*tupleType)

// check for built-in functions
Copy link
Contributor

Choose a reason for hiding this comment

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

If real and imag are added later, it would need to be handled here as well.
Can you add a comment for this?

if cx, ok := vx.Func.(*ConstExpr); ok {
if fv, ok := cx.V.(*FuncValue); ok {
if fv.PkgPath == uversePkgPath {
// TODO: should support min, max
switch {
case fv.Name == "len":
checkConstantExpr(store, last, vx.Args[0])
break Main
case fv.Name == "cap":
checkConstantExpr(store, last, vx.Args[0])
break Main
}
}
}
}

switch {
case len(tup.Elts) == 0:
panic(fmt.Sprintf("%s (no value) used as value", vx.String()))
case len(tup.Elts) == 1:
panic(fmt.Sprintf("%s (value of type %s) is not constant", vx.String(), tup.Elts[0]))
default:
panic(fmt.Sprintf("multiple-value %s (value of type %s) in single-value context", vx.String(), tup.Elts))
}
case *TypeType:
for _, arg := range vx.Args {
checkConstantExpr(store, last, arg)
}
case *NativeType:
Copy link
Contributor

Choose a reason for hiding this comment

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

a better msg? a test?

Copy link
Member Author

@omarsy omarsy Oct 23, 2024

Choose a reason for hiding this comment

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

Improved the message, but testing this branch is on hold until we resolve #3006.
related commit: b80a890

panic("NativeType\n")
default:
panic(fmt.Sprintf(
"unexpected func type %v (%v)",
ift, reflect.TypeOf(ift)))

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L266-L271

Added lines #L266 - L271 were not covered by tests
}
case *BinaryExpr:
checkConstantExpr(store, last, vx.Left)
checkConstantExpr(store, last, vx.Right)
Copy link
Contributor

Choose a reason for hiding this comment

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

consider this one as a special case:

package main

var x = 1
var y = 1

const b = x == y

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

Copy link
Member Author

Choose a reason for hiding this comment

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

this one return an error in my PR, I think. Do you want I added as a test ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

@omarsy omarsy Oct 22, 2024

Choose a reason for hiding this comment

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

I think that when you run it, you'll encounter an error like x (variable of type int) is not constant. While it's not the same error, I think the description of the error in my PR better clarifies the issue we have.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, it would be more natural that the RHS is not const, rather than its child node.

Copy link
Member Author

Choose a reason for hiding this comment

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

I find the current error message a bit unclear. For example, consider the following code:

package main

const x = 1

var y = 1

const b = x == y

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

In Go, this will produce the error: x == y (untyped bool value) is not constant. However, in the PR, the error we get is: y (variable of type int) is not constant.

This difference suggests that we should change our code to make it work, like so:

package main

const x = 1

const y = 1

const b = x == y

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

The Go error message is more informative, indicating that the expression itself is not considered constant rather than simply pointing out that y is a variable.

Copy link
Member Author

@omarsy omarsy Oct 23, 2024

Choose a reason for hiding this comment

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

But I can the logic of the code if you think it is a blocker (to be more go like)

case *SelectorExpr:
xt := evalStaticTypeOf(store, last, vx.X)
switch xt := xt.(type) {
case *PackageType:
// Todo: check if the package is const after the fix of https://github.com/gnolang/gno/issues/2836
// var pv *PackageValue
// if cx, ok := vx.X.(*ConstExpr); ok {
// // NOTE: *Machine.TestMemPackage() needs this
// // to pass in an imported package as *ConstEzpr.
// pv = cx.V.(*PackageValue)
// } else {
// // otherwise, packages can only be referred to by
// // *NameExprs, and cannot be copied.
// pvc := evalConst(store, last, vx.X)
// pv_, ok := pvc.V.(*PackageValue)
// if !ok {
// panic(fmt.Sprintf(
// "missing package in selector expr %s",
// vx.String()))
// }
// pv = pv_
// }
// if pv.GetBlock(store).Source.GetIsConst(store, vx.Sel) {
// break Main
// }
// panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.String(), xt))
omarsy marked this conversation as resolved.
Show resolved Hide resolved
case *PointerType, *DeclaredType, *StructType, *InterfaceType:
Copy link
Contributor

Choose a reason for hiding this comment

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

more tests on this?

Copy link
Member Author

Choose a reason for hiding this comment

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

right, done here: e74e6e9

ty := evalStaticTypeOf(store, last, vx.X)
panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.String(), ty))
case *TypeType:
ty := evalStaticType(store, last, vx.X)
panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.String(), ty))
case *NativeType:
panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.String(), xt))
default:
panic(fmt.Sprintf(
"unexpected selector expression type %v",
reflect.TypeOf(xt)))

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L302-L313

Added lines #L302 - L313 were not covered by tests
}

case *ArrayTypeExpr:

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L316

Added line #L316 was not covered by tests
case *ConstExpr:
case *BasicLitExpr:

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

View check run for this annotation

Codecov / codecov/patch

gnovm/pkg/gnolang/type_check.go#L318

Added line #L318 was not covered by tests
case *CompositeLitExpr:
checkConstantExpr(store, last, vx.Type)
default:
ift := evalStaticType(store, last, vx)
panic(fmt.Sprintf("%s (variable of type %s) is not constant", vx.String(), ift))
}
}

// 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: [](const-type string) (variable of type []string) is not constant
76 changes: 76 additions & 0 deletions gnovm/tests/files/const24.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
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)

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)
}

// 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
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) (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
15 changes: 15 additions & 0 deletions gnovm/tests/files/const31.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import "fmt"

func v() (string, string) {
return "", ""
}

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

// Error:
// main/files/const31.gno:10:8: multiple-value (const (v func()( string, string)))() (value of type [string string]) in single-value context
11 changes: 11 additions & 0 deletions gnovm/tests/files/const32.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package main

import "fmt"

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

// Error:
// main/files/const32.gno:6:8: [](const-type string) (variable of type []string) is not constant
Loading