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

fix: achieve type assertion parity with go #1689

Merged
merged 30 commits into from
May 21, 2024
Merged
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
4053dcf
first pass fixing type assertion bugs
deelawn Feb 23, 2024
36ef721
applied similar bug fix to native interface type assertions
deelawn Feb 23, 2024
09f9704
added file tests
deelawn Feb 27, 2024
7fce96e
code cleanup. make sure typeassert1 handles nil concrete assertions
deelawn Feb 27, 2024
2695b64
copy the tests added for typeassert2 and apply to typeassert1
deelawn Feb 27, 2024
7d4e8e6
Merge branch 'master' into bug/nil-typeassert2
deelawn Feb 27, 2024
347db5d
removed unnecessary conditional
deelawn Feb 29, 2024
e34a322
don't allow max cycle configuration
deelawn Mar 27, 2024
2866d97
Revert "don't allow max cycle configuration"
deelawn Mar 27, 2024
7136f30
Merge remote-tracking branch 'upstream/master'
deelawn Mar 29, 2024
355b8e8
Merge remote-tracking branch 'upstream/master'
deelawn Apr 8, 2024
1db1c4c
variable rename
deelawn Apr 8, 2024
eca217b
Merge remote-tracking branch 'upstream/master'
deelawn Apr 10, 2024
d9851da
added basic wildcard matching for filetest errors
deelawn Apr 16, 2024
fc58beb
only allow type assertions on interface types
deelawn Apr 16, 2024
5074eba
don't allow type assertions of the blank identifer
deelawn Apr 16, 2024
2efb7b8
panic on nil interface assertions
deelawn Apr 16, 2024
ce6b12d
verify expression type
deelawn Apr 16, 2024
bd10bac
comment
deelawn Apr 16, 2024
23f13fb
Merge branch 'master' of github.com:gnolang/gno
deelawn Apr 16, 2024
5867ba9
rename tests added
deelawn Apr 16, 2024
9ac8384
Merge branch 'master' into bug/nil-typeassert2
deelawn Apr 16, 2024
6196fdc
account for native interface types in type assertion preprocessing
deelawn Apr 16, 2024
bd65ea2
restore moved test
deelawn Apr 16, 2024
ec58faa
removed dup tests and fixed some
deelawn Apr 16, 2024
c52c243
fixed error message
deelawn Apr 16, 2024
3301ffe
updated tests error message
deelawn Apr 16, 2024
cb2a92b
undid stupid idea
deelawn Apr 18, 2024
7685853
added test
deelawn Apr 23, 2024
7ab7db5
Merge branch 'master' into bug/nil-typeassert2
deelawn May 21, 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
107 changes: 89 additions & 18 deletions gnovm/pkg/gnolang/op_expressions.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,41 @@ func (m *Machine) doOpRef() {
func (m *Machine) doOpTypeAssert1() {
m.PopExpr()
// pop type
t := m.PopValue().GetType()
t := m.PopValue().GetType() // type being asserted

// peek x for re-use
xv := m.PeekValue(1)
xt := xv.T
xv := m.PeekValue(1) // value result / value to assert
xt := xv.T // underlying value's type

// xt may be nil, but we need to wait to return because the value of xt that is set
// will depend on whether we are trying to assert to an interface or concrete type.
// xt can be nil in the case where recover can't find a panic to recover from and
// returns a bare TypedValue{}.

if t.Kind() == InterfaceKind { // is interface assert
if xt == nil || xv.IsNilInterface() {
// TODO: default panic type?
ex := fmt.Sprintf("interface conversion: interface is nil, not %s", t.String())
m.Panic(typedString(ex))
return
}

if it, ok := baseOf(t).(*InterfaceType); ok {
// An interface type assertion on a value that doesn't have a concrete base
// type should always fail.
if _, ok := baseOf(xt).(*InterfaceType); ok {
// TODO: default panic type?
ex := fmt.Sprintf(
deelawn marked this conversation as resolved.
Show resolved Hide resolved
"non-concrete %s doesn't implement %s",
xt.String(),
it.String())
m.Panic(typedString(ex))
return
}

thehowl marked this conversation as resolved.
Show resolved Hide resolved
// t is Gno interface.
// assert that x implements type.
impl := false
var impl bool
impl = it.IsImplementedBy(xt)
if !impl {
// TODO: default panic type?
Expand All @@ -230,16 +255,22 @@ func (m *Machine) doOpTypeAssert1() {
} else if nt, ok := baseOf(t).(*NativeType); ok {
// t is Go interface.
// assert that x implements type.
impl := false
errPrefix := "non-concrete "
var impl bool
if nxt, ok := xt.(*NativeType); ok {
impl = nxt.Type.Implements(nt.Type)
} else {
impl = false
// If the underlying native type is reflect.Interface kind, then this has no
// concrete value and should fail.
if nxt.Type.Kind() != reflect.Interface {
impl = nxt.Type.Implements(nt.Type)
errPrefix = ""
}
}

if !impl {
// TODO: default panic type?
ex := fmt.Sprintf(
"%s doesn't implement %s",
"%s%s doesn't implement %s",
errPrefix,
xt.String(),
nt.String())
m.Panic(typedString(ex))
Expand All @@ -251,6 +282,12 @@ func (m *Machine) doOpTypeAssert1() {
panic("should not happen")
}
} else { // is concrete assert
if xt == nil {
ex := fmt.Sprintf("nil is not of type %s", t.String())
m.Panic(typedString(ex))
return
}

tid := t.TypeID()
xtid := xt.TypeID()
// assert that x is of type.
Expand All @@ -273,17 +310,37 @@ func (m *Machine) doOpTypeAssert1() {
func (m *Machine) doOpTypeAssert2() {
m.PopExpr()
// peek type for re-use
tv := m.PeekValue(1)
t := tv.GetType()
tv := m.PeekValue(1) // boolean result
t := tv.GetType() // type being asserted

// peek x for re-use
xv := m.PeekValue(2)
xt := xv.T
xv := m.PeekValue(2) // value result / value to assert
xt := xv.T // underlying value's type

// xt may be nil, but we need to wait to return because the value of xt that is set
// will depend on whether we are trying to assert to an interface or concrete type.
// xt can be nil in the case where recover can't find a panic to recover from and
// returns a bare TypedValue{}.

if t.Kind() == InterfaceKind { // is interface assert
if xt == nil {
*xv = TypedValue{}
*tv = untypedBool(false)
return
}

if it, ok := baseOf(t).(*InterfaceType); ok {
// An interface type assertion on a value that doesn't have a concrete base
// type should always fail.
if _, ok := baseOf(xt).(*InterfaceType); ok {
*xv = TypedValue{}
*tv = untypedBool(false)
return
}

// t is Gno interface.
// assert that x implements type.
impl := false
var impl bool
impl = it.IsImplementedBy(xt)
if impl {
// *xv = *xv
Expand All @@ -295,14 +352,18 @@ func (m *Machine) doOpTypeAssert2() {
*tv = untypedBool(false)
}
} else if nt, ok := baseOf(t).(*NativeType); ok {
// If the value being asserted on is nil, it can't implement an interface.
// t is Go interface.
// assert that x implements type.
impl := false
var impl bool
if nxt, ok := xt.(*NativeType); ok {
impl = nxt.Type.Implements(nt.Type)
} else {
impl = false
// If the underlying native type is reflect.Interface kind, then this has no
// concrete value and should fail.
if nxt.Type.Kind() != reflect.Interface {
impl = nxt.Type.Implements(nt.Type)
}
}

if impl {
// *xv = *xv
*tv = untypedBool(true)
Expand All @@ -314,10 +375,20 @@ func (m *Machine) doOpTypeAssert2() {
panic("should not happen")
}
} else { // is concrete assert
if xt == nil {
*xv = TypedValue{
T: t,
V: defaultValue(m.Alloc, t),
}
*tv = untypedBool(false)
return
}

tid := t.TypeID()
xtid := xt.TypeID()
// assert that x is of type.
same := tid == xtid

if same {
// *xv = *xv
*tv = untypedBool(true)
Expand Down
27 changes: 27 additions & 0 deletions gnovm/pkg/gnolang/preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/gnolang/gno/tm2/pkg/errors"
)

const blankIdentifer string = "_"
deelawn marked this conversation as resolved.
Show resolved Hide resolved

// In the case of a *FileSet, some declaration steps have to happen
// in a restricted parallel way across all the files.
// Anything predefined or preprocessed here get skipped during the Preprocess
Expand Down Expand Up @@ -1237,8 +1239,33 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
if n.Type == nil {
panic("should not happen")
}

// Type assertions on the blank identifier are illegal.
if nx, ok := n.X.(*NameExpr); ok && string(nx.Name) == blankIdentifer {
deelawn marked this conversation as resolved.
Show resolved Hide resolved
panic("cannot use _ as value or type")
}

// ExprStmt of form `x.(<type>)`,
// or special case form `c, ok := x.(<type>)`.
t := evalStaticTypeOf(store, last, n.X)
baseType := baseOf(t) // The base type of the asserted value must be an interface.
switch bt := baseType.(type) {
case *InterfaceType:
break
case *NativeType:
if bt.Type.Kind() == reflect.Interface {
break
}
default:
panic(
fmt.Sprintf(
"invalid operation: %s (variable of type %s) is not an interface",
n.X.String(),
t.String(),
),
)
}

evalStaticType(store, last, n.Type)

// TRANS_LEAVE -----------------------
Expand Down
1 change: 1 addition & 0 deletions gnovm/tests/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
default:
errstr = strings.TrimSpace(fmt.Sprintf("%v", pnc))
}

if errstr != errWanted {
panic(fmt.Sprintf("fail on %s: got %q, want: %q", path, errstr, errWanted))
}
Expand Down
13 changes: 13 additions & 0 deletions gnovm/tests/files/typeassert1.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package main

type A interface {
Do(s string)
}

func main() {
var a A
_ = a.(A)
}

// Error:
// interface conversion: interface is nil, not main.A
32 changes: 32 additions & 0 deletions gnovm/tests/files/typeassert2.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package main

type ex int

func (ex) Error() string { return "" }

type A interface {
Do(s string)
}

func main() {
defer func() {
e := recover()
if _, ok := e.(ex); ok {
println("wat")
} else {
println("ok")
}
}()
defer func() {
e := recover()
if _, ok := e.(A); ok {
println("wat")
} else {
println("ok")
}
}()
}

// Output:
// ok
// ok
15 changes: 15 additions & 0 deletions gnovm/tests/files/typeassert2a.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

type A interface {
Do(s string)
}

func main() {
defer func() {
e := recover()
_ = e.(A)
}()
}

// Error:
// interface conversion: interface is nil, not main.A
15 changes: 15 additions & 0 deletions gnovm/tests/files/typeassert3.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

type ex int

func (ex) Error() string { return "" }

func main() {
defer func() {
r := _.(ex)
println(r)
}()
}

// Error:
// main/files/typeassert3.gno:9: cannot use _ as value or type
58 changes: 58 additions & 0 deletions gnovm/tests/files/typeassert4.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package main

type Setter interface {
Set(string)
}

type SetterClone interface {
Set(string)
}

type ValueSetter struct {
value string
}

func (s *ValueSetter) Set(value string) {
s.value = value
}

func cmpSetter(i interface{}) {
if _, ok := i.(Setter); ok {
println("ok")
} else {
println("not ok")
}
}

func main() {
var (
i interface{}
setter Setter
setterClone SetterClone
valueSetter ValueSetter
valueSetterPtr *ValueSetter
)

cmpSetter(i)

i = setter
cmpSetter(i)

setterClone = valueSetterPtr
setter = setterClone
i = setter
cmpSetter(i)

i = valueSetter
cmpSetter(i)

i = valueSetterPtr
cmpSetter(i)
}

// Output:
// not ok
// not ok
// ok
// not ok
// ok
Loading
Loading