From 63825e720100fbcd1363eb3bb8e957212efbd152 Mon Sep 17 00:00:00 2001 From: Marc Vertes Date: Thu, 1 Sep 2022 12:18:08 +0200 Subject: [PATCH] interp: fix use of interfaces in composite types 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. --- _test/assert2.go | 4 ++-- _test/issue-1447.go | 20 ++++++++++++++++++++ interp/run.go | 21 ++++++++------------- interp/type.go | 32 +++++++++++++++++++++++++------- interp/typecheck.go | 2 +- interp/value.go | 17 ++--------------- 6 files changed, 58 insertions(+), 38 deletions(-) create mode 100644 _test/issue-1447.go diff --git a/_test/assert2.go b/_test/assert2.go index 824efbe5..988c353d 100644 --- a/_test/assert2.go +++ b/_test/assert2.go @@ -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) diff --git a/_test/issue-1447.go b/_test/issue-1447.go new file mode 100644 index 00000000..1878f476 --- /dev/null +++ b/_test/issue-1447.go @@ -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[]} diff --git a/interp/run.go b/interp/run.go index 3cedc6f8..7660d1df 100644 --- a/interp/run.go +++ b/interp/run.go @@ -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()) @@ -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() } @@ -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()})) @@ -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): @@ -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): @@ -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. @@ -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)) @@ -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) diff --git a/interp/type.go b/interp/type.go index fbc4299d..9cc7191c 100644 --- a/interp/type.go +++ b/interp/type.go @@ -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 { @@ -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: @@ -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: @@ -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()) @@ -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 } @@ -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 { diff --git a/interp/typecheck.go b/interp/typecheck.go index b673bbac..e0473cbf 100644 --- a/interp/typecheck.go +++ b/interp/typecheck.go @@ -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() diff --git a/interp/value.go b/interp/value.go index fe7ed98c..7ecf484a 100644 --- a/interp/value.go +++ b/interp/value.go @@ -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: @@ -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) @@ -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}) }