Skip to content

Commit

Permalink
fix: achieve type assertion parity with go (#1689)
Browse files Browse the repository at this point in the history
<!-- please provide a detailed description of the changes made in this
pull request. -->
Fixes #1650 

This PR was originally intended to fix the case described in issue
#1650, but it soon became apparent that there were other subtle
inconsistencies between type assertions in go vs gno. The changes
outlined here attempt to fix the issues and the new file tests ensure
correctness. The summary below provides details as to the cases that
were fixed for type assertions, both 1 and 2 -- 1 and 2 referring to the
type assertion op codes, `OpTypeAssert1` and `OpTypeAssert2`. The former
handles type assertions with one LHS argument that panic on assertion
failure and the latter handles those with two LHS arguments that assigns
a boolean value dependent on success or failure.

### Summary of type assertion changes
- fail when the value being asserted has a nil type (i.e. the result of
`recover()` when no panic has occurred). This applies for both cases
where the type being asserted to is an interface or concrete type.
- fail when asserting to an interface type and the underlying type of
the value being asserted is also an interface type (non-concrete type)

Also, is this an accurate assumption?

https://github.com/gnolang/gno/blob/7d4e8e645ca1d2e89f5a8f2e85470e66b3253b33/gnovm/pkg/gnolang/op_expressions.go#L261

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

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] 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>
  • Loading branch information
deelawn committed May 21, 2024
1 parent fc9db26 commit 7d939a0
Show file tree
Hide file tree
Showing 18 changed files with 579 additions and 18 deletions.
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(
"non-concrete %s doesn't implement %s",
xt.String(),
it.String())
m.Panic(typedString(ex))
return
}

// 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 = "_"

// 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 {
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

0 comments on commit 7d939a0

Please sign in to comment.