Skip to content

Commit

Permalink
interp: fix use of interfaces in composite types
Browse files Browse the repository at this point in the history
The representation of non empty interfaces defined in the interpreter is now identical between refType() and frameType() functions, which are used to generate interpreter objects.

Fixes #1447 and #1426.
  • Loading branch information
mvertes authored Sep 1, 2022
1 parent 03ccda1 commit 63825e7
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 38 deletions.
4 changes: 2 additions & 2 deletions _test/assert2.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"sync"
)

// Defined an interface of stringBuilder that compatible with
// strings.Builder(go 1.10) and bytes.Buffer(< go 1.10)
// Define an interface of stringBuilder that is compatible with
// strings.Builder(go 1.10) and bytes.Buffer(< go 1.10).
type stringBuilder interface {
WriteRune(r rune) (n int, err error)
WriteString(s string) (int, error)
Expand Down
20 changes: 20 additions & 0 deletions _test/issue-1447.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package main

import "fmt"

type I interface {
Name() string
}

type S struct {
iMap map[string]I
}

func main() {
s := S{}
s.iMap = map[string]I{}
fmt.Println(s)
}

// Output:
// {map[]}
21 changes: 8 additions & 13 deletions interp/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ func convert(n *node) {
if c.isNil() { // convert nil to type
// TODO(mpl): Try to completely remove, as maybe frameType already does the job for interfaces.
if isInterfaceSrc(n.child[0].typ) && !isEmptyInterface(n.child[0].typ) {
typ = reflect.TypeOf((*valueInterface)(nil)).Elem()
typ = valueInterfaceType
}
n.exec = func(f *frame) bltn {
dest(f).Set(reflect.New(typ).Elem())
Expand Down Expand Up @@ -713,7 +713,7 @@ func assign(n *node) {
case isFuncSrc(typ):
t = reflect.TypeOf((*node)(nil))
case isInterfaceSrc(typ):
t = reflect.TypeOf((*valueInterface)(nil)).Elem()
t = valueInterfaceType
default:
t = typ.TypeOf()
}
Expand Down Expand Up @@ -1007,7 +1007,7 @@ func genFunctionWrapper(n *node) func(*frame) reflect.Value {
}
typ := def.typ.arg[i]
switch {
case isEmptyInterface(typ):
case isEmptyInterface(typ) || typ.TypeOf() == valueInterfaceType:
d[i].Set(arg)
case isInterfaceSrc(typ):
d[i].Set(reflect.ValueOf(valueInterface{value: arg.Elem()}))
Expand Down Expand Up @@ -1560,12 +1560,9 @@ func callBin(n *node) {
case isInterfaceSrc(c.typ):
values = append(values, genValueInterfaceValue(c))
case c.typ.cat == arrayT || c.typ.cat == variadicT:
switch {
case isEmptyInterface(c.typ.val):
if isEmptyInterface(c.typ.val) {
values = append(values, genValueArray(c))
case isInterfaceSrc(c.typ.val):
values = append(values, genValueInterfaceArray(c))
default:
} else {
values = append(values, genInterfaceWrapper(c, defType))
}
case isPtrSrc(c.typ):
Expand Down Expand Up @@ -2703,8 +2700,6 @@ func doComposite(n *node, hasType bool, keyed bool) {
values[fieldIndex] = func(*frame) reflect.Value { return reflect.New(rft).Elem() }
case isFuncSrc(val.typ):
values[fieldIndex] = genValueAsFunctionWrapper(val)
case isArray(val.typ) && val.typ.val != nil && isInterfaceSrc(val.typ.val) && !isEmptyInterface(val.typ.val):
values[fieldIndex] = genValueInterfaceArray(val)
case isInterfaceSrc(ft) && (!isEmptyInterface(ft) || len(val.typ.method) > 0):
values[fieldIndex] = genValueInterface(val)
case isInterface(ft):
Expand Down Expand Up @@ -3585,7 +3580,7 @@ func convertLiteralValue(n *node, t reflect.Type) {
case n.typ.cat == nilT:
// Create a zero value of target type.
n.rval = reflect.New(t).Elem()
case !(n.kind == basicLit || n.rval.IsValid()) || t == nil || t.Kind() == reflect.Interface || t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Interface:
case !(n.kind == basicLit || n.rval.IsValid()) || t == nil || t.Kind() == reflect.Interface || t == valueInterfaceType || t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Interface:
// Skip non-constant values, undefined target type or interface target type.
case n.rval.IsValid():
// Convert constant value to target type.
Expand Down Expand Up @@ -3946,7 +3941,7 @@ func isNotNil(n *node) {
dest := genValue(n)

if n.fnext == nil {
if isInterfaceSrc(c0.typ) {
if isInterfaceSrc(c0.typ) && c0.typ.TypeOf() != valueInterfaceType {
if isInterface {
n.exec = func(f *frame) bltn {
dest(f).Set(reflect.ValueOf(!value(f).IsNil()).Convert(typ))
Expand Down Expand Up @@ -3991,7 +3986,7 @@ func isNotNil(n *node) {

fnext := getExec(n.fnext)

if isInterfaceSrc(c0.typ) {
if isInterfaceSrc(c0.typ) && c0.typ.TypeOf() != valueInterfaceType {
n.exec = func(f *frame) bltn {
if value(f).IsNil() {
dest(f).SetBool(false)
Expand Down
32 changes: 25 additions & 7 deletions interp/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -1817,8 +1817,9 @@ func exportName(s string) string {

var (
// TODO(mpl): generators.
interf = reflect.TypeOf((*interface{})(nil)).Elem()
constVal = reflect.TypeOf((*constant.Value)(nil)).Elem()
emptyInterfaceType = reflect.TypeOf((*interface{})(nil)).Elem()
valueInterfaceType = reflect.TypeOf((*valueInterface)(nil)).Elem()
constVal = reflect.TypeOf((*constant.Value)(nil)).Elem()
)

type fieldRebuild struct {
Expand Down Expand Up @@ -1971,7 +1972,12 @@ func (t *itype) refType(ctx *refTypeContext) reflect.Type {
}
t.rtype = reflect.FuncOf(in, out, variadic)
case interfaceT:
t.rtype = interf
if len(t.field) == 0 {
// empty interface, do not wrap it
t.rtype = emptyInterfaceType
break
}
t.rtype = valueInterfaceType
case mapT:
t.rtype = reflect.MapOf(t.key.refType(ctx), t.val.refType(ctx))
case ptrT:
Expand Down Expand Up @@ -2056,10 +2062,10 @@ func (t *itype) frameType() (r reflect.Type) {
case interfaceT:
if len(t.field) == 0 {
// empty interface, do not wrap it
r = reflect.TypeOf((*interface{})(nil)).Elem()
r = emptyInterfaceType
break
}
r = reflect.TypeOf((*valueInterface)(nil)).Elem()
r = valueInterfaceType
case mapT:
r = reflect.MapOf(t.key.frameType(), t.val.frameType())
case ptrT:
Expand All @@ -2072,6 +2078,14 @@ func (t *itype) frameType() (r reflect.Type) {

func (t *itype) implements(it *itype) bool {
if isBin(t) {
// Note: in case of a valueInterfaceType, we
// miss required data which will be available
// later, so we optimistically return true to progress,
// and additional checks will be hopefully performed at
// runtime.
if rt := it.TypeOf(); rt == valueInterfaceType {
return true
}
return t.TypeOf().Implements(it.TypeOf())
}
return t.methods().contains(it.methods())
Expand Down Expand Up @@ -2127,11 +2141,15 @@ func (t *itype) defaultType(v reflect.Value, sc *scope) *itype {
func (t *itype) isNil() bool { return t.cat == nilT }

func (t *itype) hasNil() bool {
switch t.TypeOf().Kind() {
switch rt := t.TypeOf(); rt.Kind() {
case reflect.UnsafePointer:
return true
case reflect.Slice, reflect.Ptr, reflect.Func, reflect.Interface, reflect.Map, reflect.Chan:
return true
case reflect.Struct:
if rt == valueInterfaceType {
return true
}
}
return false
}
Expand Down Expand Up @@ -2246,7 +2264,7 @@ func isInterfaceBin(t *itype) bool {
}

func isInterface(t *itype) bool {
return isInterfaceSrc(t) || t.TypeOf() != nil && t.TypeOf().Kind() == reflect.Interface
return isInterfaceSrc(t) || t.TypeOf() == valueInterfaceType || t.TypeOf() != nil && t.TypeOf().Kind() == reflect.Interface
}

func isBin(t *itype) bool {
Expand Down
2 changes: 1 addition & 1 deletion interp/typecheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ func (check typecheck) typeAssertionExpr(n *node, typ *itype) error {
// https://github.com/golang/go/issues/39717 lands. It is currently impractical to
// type check Named types as they cannot be asserted.

if n.typ.TypeOf().Kind() != reflect.Interface {
if rt := n.typ.TypeOf(); rt.Kind() != reflect.Interface && rt != valueInterfaceType {
return n.cfgErrorf("invalid type assertion: non-interface type %s on left", n.typ.id())
}
ims := n.typ.methods()
Expand Down
17 changes: 2 additions & 15 deletions interp/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func genValue(n *node) func(*frame) reflect.Value {
convertConstantValue(n)
v := n.rval
if !v.IsValid() {
v = reflect.New(interf).Elem()
v = reflect.New(emptyInterfaceType).Elem()
}
return func(f *frame) reflect.Value { return v }
case funcDecl:
Expand Down Expand Up @@ -287,19 +287,6 @@ func genValueRangeArray(n *node) func(*frame) reflect.Value {
}
}

func genValueInterfaceArray(n *node) func(*frame) reflect.Value {
value := genValue(n)
return func(f *frame) reflect.Value {
vi := value(f).Interface().([]valueInterface)
v := reflect.MakeSlice(reflect.TypeOf([]interface{}{}), len(vi), len(vi))
for i, vv := range vi {
v.Index(i).Set(vv.value)
}

return v
}
}

func genValueInterface(n *node) func(*frame) reflect.Value {
value := genValue(n)

Expand Down Expand Up @@ -356,7 +343,7 @@ func getConcreteValue(val reflect.Value) reflect.Value {

func zeroInterfaceValue() reflect.Value {
n := &node{kind: basicLit, typ: &itype{cat: nilT, untyped: true, str: "nil"}}
v := reflect.New(interf).Elem()
v := reflect.New(emptyInterfaceType).Elem()
return reflect.ValueOf(valueInterface{n, v})
}

Expand Down

0 comments on commit 63825e7

Please sign in to comment.