gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + Gno.land: a blockchain for timeless code and fair open-source
https://gno.land/
Other
849 stars 345 forks source link

println does not print nil slices #1377

Closed thehowl closed 7 months ago

thehowl commented 7 months ago
package main

func main() {
        var a []string
        println(a)
}

panics with:

panic: interface conversion: gnolang.Value is nil, not *gnolang.SliceValue [recovered]
        panic: interface conversion: gnolang.Value is nil, not *gnolang.SliceValue

goroutine 1 [running]:
main.runExpr.func1()
        /home/howl/oc/gno2/gnovm/cmd/gno/run.go:158 +0x125
panic({0xc0ae00?, 0xc00047c840?})
        /usr/lib/go/src/runtime/panic.go:914 +0x21f
github.com/gnolang/gno/gnovm/pkg/gnolang.(*TypedValue).Sprint(0xc0001218c8, 0xc00047c780?)
        /home/howl/oc/gno2/gnovm/pkg/gnolang/values_string.go:230 +0xe71
github.com/gnolang/gno/gnovm/pkg/gnolang.UverseNode.func11(0xc000177200)
        /home/howl/oc/gno2/gnovm/pkg/gnolang/uverse.go:947 +0x1ee
github.com/gnolang/gno/gnovm/pkg/gnolang.(*Machine).doOpCallNativeBody(...)
        /home/howl/oc/gno2/gnovm/pkg/gnolang/op_call.go:166
notJoon commented 7 months ago

The section in gnolang where println is called is from lines 935 to 955 in uverse.go. It seems to be an issue caused by not handling nil values in this part. However, when the same code is run in Go, it outputs [0/0]0x0. Should we modify it to simply print undefined instead?

thehowl commented 7 months ago

Should we modify it to simply print undefined instead?

Yeah, sounds adequate.

ajnavarro commented 7 months ago

@thehowl do we want to modify standard Go behavior on Gno changing [0/0]0x0 output to undefined?

IMHO we should default to keep the same behavior unless there is a good reasoning behind the change. We cannot know the implications of most of the changes until we hit the problem eventually.

thehowl commented 7 months ago

@ajnavarro println and print are not properly "standardized" function where we should strive to have 100% compatibility with Go. See: Go spec#Bootstrapping, builtin#println. The documentation explicitly says:

The println built-in function formats its arguments in an implementation-specific way

(emphasis mine)

I think printing undefined is acceptable as it is consistent with the behaviour when you perform println(nil).

thehowl commented 7 months ago

Fixed by #1380

(@notJoon use "linking keywords" for your next PRs; see github doc)