diff --git a/ssa/builder.go b/ssa/builder.go index 8fc5de6a..bf52f21c 100644 --- a/ssa/builder.go +++ b/ssa/builder.go @@ -523,6 +523,8 @@ func (b *builder) expr(fn *Function, e ast.Expr) Value { switch y := y.(type) { case *Convert: y.pos = e.Lparen + case *ChangeInterface: + y.pos = e.Lparen case *ChangeType: y.pos = e.Lparen case *MakeInterface: @@ -662,8 +664,6 @@ func (b *builder) expr(fn *Function, e ast.Expr) Value { wantAddr := isPointer(recvType(obj)) escaping := true v := b.receiver(fn, e.X, wantAddr, escaping, indices, isIndirect) - // TODO(adonovan): test the case where - // *struct{I} inherits a method from interface I. c := &MakeClosure{ Fn: boundMethodWrapper(fn.Prog, obj), Bindings: []Value{v}, @@ -761,7 +761,11 @@ func (b *builder) receiver(fn *Function, e ast.Expr, wantAddr, escaping bool, in } last := len(indices) - 1 - return emitImplicitSelections(fn, v, indices[:last]) + v = emitImplicitSelections(fn, v, indices[:last]) + if !wantAddr && isPointer(v.Type()) { + v = emitLoad(fn, v) + } + return v } // setCallFunc populates the function parts of a CallCommon structure @@ -836,10 +840,6 @@ func (b *builder) setCallFunc(fn *Function, e *ast.CallExpr, c *CallCommon) { escaping := true v := b.receiver(fn, sel.X, wantAddr, escaping, indices, isIndirect) if _, ok := deref(v.Type()).Underlying().(*types.Interface); ok { - if isPointer(v.Type()) { - // *struct{I} inherits methods from I. - v = emitLoad(fn, v) - } // Invoke-mode call. c.Value = v c.Method = obj @@ -1540,18 +1540,23 @@ func (b *builder) selectStmt(fn *Function, s *ast.SelectStmt, label *lblock) { Chan: ch, Send: emitConv(fn, b.expr(fn, comm.Value), ch.Type().Underlying().(*types.Chan).Elem()), + Pos: comm.Arrow, }) case *ast.AssignStmt: // x := <-ch + unary := unparen(comm.Rhs[0]).(*ast.UnaryExpr) states = append(states, SelectState{ Dir: ast.RECV, - Chan: b.expr(fn, unparen(comm.Rhs[0]).(*ast.UnaryExpr).X), + Chan: b.expr(fn, unary.X), + Pos: unary.OpPos, }) case *ast.ExprStmt: // <-ch + unary := unparen(comm.X).(*ast.UnaryExpr) states = append(states, SelectState{ Dir: ast.RECV, - Chan: b.expr(fn, unparen(comm.X).(*ast.UnaryExpr).X), + Chan: b.expr(fn, unary.X), + Pos: unary.OpPos, }) } } diff --git a/ssa/emit.go b/ssa/emit.go index 1e09eeb1..130721d0 100644 --- a/ssa/emit.go +++ b/ssa/emit.go @@ -160,7 +160,7 @@ func isValuePreserving(ut_src, ut_dst types.Type) bool { // emitConv emits to f code to convert Value val to exactly type typ, // and returns the converted value. Implicit conversions are required // by language assignability rules in assignments, parameter passing, -// etc. +// etc. Conversions cannot fail dynamically. // func emitConv(f *Function, val Value, typ types.Type) Value { t_src := val.Type() @@ -185,7 +185,9 @@ func emitConv(f *Function, val Value, typ types.Type) Value { // Assignment from one interface type to another? if _, ok := ut_src.(*types.Interface); ok { - return emitTypeAssert(f, val, typ, token.NoPos) + c := &ChangeInterface{X: val} + c.setType(typ) + return f.emit(c) } // Untyped nil constant? Return interface-typed nil constant. @@ -268,20 +270,6 @@ func emitExtract(f *Function, tuple Value, index int, typ types.Type) Value { // returns the value. x.Type() must be an interface. // func emitTypeAssert(f *Function, x Value, t types.Type, pos token.Pos) Value { - // Simplify infallible assertions. - txi := x.Type().Underlying().(*types.Interface) - if ti, ok := t.Underlying().(*types.Interface); ok { - // Even when ti==txi, we still need ChangeInterface - // since it performs a nil-check. - // TODO(adonovan): needs more tests. - if types.IsAssignableTo(ti, txi) { - c := &ChangeInterface{X: x} - c.setPos(pos) - c.setType(t) - return f.emit(c) - } - } - a := &TypeAssert{X: x, AssertedType: t} a.setPos(pos) a.setType(t) @@ -292,10 +280,6 @@ func emitTypeAssert(f *Function, x Value, t types.Type, pos token.Pos) Value { // a (value, ok) tuple. x.Type() must be an interface. // func emitTypeTest(f *Function, x Value, t types.Type, pos token.Pos) Value { - // TODO(adonovan): opt: simplify infallible tests as per - // emitTypeAssert, and return (x, vTrue). - // (Requires that exprN returns a slice of extracted values, - // not a single Value of type *types.Tuple.) a := &TypeAssert{ X: x, AssertedType: t, diff --git a/ssa/interp/interp.go b/ssa/interp/interp.go index b1aee540..e7e891cc 100644 --- a/ssa/interp/interp.go +++ b/ssa/interp/interp.go @@ -164,11 +164,7 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation { fr.env[instr] = call(fr.i, fr, instr.Pos(), fn, args) case *ssa.ChangeInterface: - x := fr.get(instr.X) - if x.(iface).t == nil { - panic(fmt.Sprintf("interface conversion: interface is nil, not %s", instr.Type())) - } - fr.env[instr] = x + fr.env[instr] = fr.get(instr.X) case *ssa.ChangeType: fr.env[instr] = fr.get(instr.X) // (can't fail) diff --git a/ssa/interp/interp_test.go b/ssa/interp/interp_test.go index 553ae679..06845a39 100644 --- a/ssa/interp/interp_test.go +++ b/ssa/interp/interp_test.go @@ -129,11 +129,13 @@ var gorootTests = []string{ // These are files in go.tools/ssa/interp/testdata/. var testdataTests = []string{ - "coverage.go", - "mrvchain.go", "boundmeth.go", - "ifaceprom.go", + "coverage.go", "fieldprom.go", + "ifaceconv.go", + "ifaceprom.go", + "methprom.go", + "mrvchain.go", } func run(t *testing.T, dir, input string) bool { diff --git a/ssa/interp/ops.go b/ssa/interp/ops.go index fbab4286..a2b260db 100644 --- a/ssa/interp/ops.go +++ b/ssa/interp/ops.go @@ -839,8 +839,12 @@ func typeAssert(i *interpreter, instr *ssa.TypeAssert, itf iface) value { var v value err := "" if idst, ok := instr.AssertedType.Underlying().(*types.Interface); ok { - v = itf - err = checkInterface(i, idst, itf) + if itf.t == nil { + err = fmt.Sprintf("interface conversion: interface is nil, not %s", instr.AssertedType) + } else { + v = itf + err = checkInterface(i, idst, itf) + } } else if types.IsIdentical(itf.t, instr.AssertedType) { v = copyVal(itf.v) // extract value diff --git a/ssa/interp/testdata/ifaceconv.go b/ssa/interp/testdata/ifaceconv.go new file mode 100644 index 00000000..96fc1058 --- /dev/null +++ b/ssa/interp/testdata/ifaceconv.go @@ -0,0 +1,83 @@ +package main + +// Tests of interface conversions and type assertions. + +type I0 interface { +} +type I1 interface { + f() +} +type I2 interface { + f() + g() +} + +type C0 struct{} +type C1 struct{} + +func (C1) f() {} + +type C2 struct{} + +func (C2) f() {} +func (C2) g() {} + +func main() { + var i0 I0 + var i1 I1 + var i2 I2 + + // Nil always causes a type assertion to fail, even to the + // same type. + if _, ok := i0.(I0); ok { + panic("nil i0.(I0) succeeded") + } + if _, ok := i1.(I1); ok { + panic("nil i1.(I1) succeeded") + } + if _, ok := i2.(I2); ok { + panic("nil i2.(I2) succeeded") + } + + // Conversions can't fail, even with nil. + _ = I0(i0) + + _ = I0(i1) + _ = I1(i1) + + _ = I0(i2) + _ = I1(i2) + _ = I2(i2) + + // Non-nil type assertions pass or fail based on the concrete type. + i1 = C1{} + if _, ok := i1.(I0); !ok { + panic("C1 i1.(I0) failed") + } + if _, ok := i1.(I1); !ok { + panic("C1 i1.(I1) failed") + } + if _, ok := i1.(I2); ok { + panic("C1 i1.(I2) succeeded") + } + + i1 = C2{} + if _, ok := i1.(I0); !ok { + panic("C2 i1.(I0) failed") + } + if _, ok := i1.(I1); !ok { + panic("C2 i1.(I1) failed") + } + if _, ok := i1.(I2); !ok { + panic("C2 i1.(I2) failed") + } + + // Conversions can't fail. + i1 = C1{} + if I0(i1) == nil { + panic("C1 I0(i1) was nil") + } + if I1(i1) == nil { + panic("C1 I1(i1) was nil") + } +} diff --git a/ssa/interp/testdata/methprom.go b/ssa/interp/testdata/methprom.go new file mode 100644 index 00000000..e8e384c3 --- /dev/null +++ b/ssa/interp/testdata/methprom.go @@ -0,0 +1,93 @@ +package main + +// Tests of method promotion logic. + +type A struct{ magic int } + +func (a A) x() { + if a.magic != 1 { + panic(a.magic) + } +} +func (a *A) y() *A { + return a +} + +type B struct{ magic int } + +func (b B) p() { + if b.magic != 2 { + panic(b.magic) + } +} +func (b *B) q() { + if b != theC.B { + panic("oops") + } +} + +type I interface { + f() +} + +type impl struct{ magic int } + +func (i impl) f() { + if i.magic != 3 { + panic("oops") + } +} + +type C struct { + A + *B + I +} + +func assert(cond bool) { + if !cond { + panic("failed") + } +} + +var theC = C{ + A: A{1}, + B: &B{2}, + I: impl{3}, +} + +func addr() *C { + return &theC +} + +func value() C { + return theC +} + +func main() { + // address + addr().x() + if addr().y() != &theC.A { + panic("oops") + } + addr().p() + addr().q() + addr().f() + + // addressable value + var c C = value() + c.x() + if c.y() != &c.A { + panic("oops") + } + c.p() + c.q() + c.f() + + // non-addressable value + value().x() + // value().y() // not in method set + value().p() + value().q() + value().f() +} diff --git a/ssa/ssa.go b/ssa/ssa.go index 4e2bfc22..29a5d645 100644 --- a/ssa/ssa.go +++ b/ssa/ssa.go @@ -582,10 +582,7 @@ type Convert struct { // ChangeInterface constructs a value of one interface type from a // value of another interface type known to be assignable to it. -// -// This operation fails if the operand is nil. -// For all other operands, well-typedness ensures success. -// Use TypeAssert for interface conversions that are uncertain. +// This operation cannot fail. // // Pos() returns the ast.CallExpr.Lparen if the instruction arose from // an explicit T(e) conversion; the ast.TypeAssertExpr.Lparen if the @@ -813,6 +810,7 @@ type SelectState struct { Dir ast.ChanDir // direction of case Chan Value // channel to use (for send or receive) Send Value // value to send (for send) + Pos token.Pos // position of token.ARROW } // The Select instruction tests whether (or blocks until) one or more @@ -915,8 +913,9 @@ type Next struct { // If AssertedType is an interface, TypeAssert checks whether the // dynamic type of the interface is assignable to it, and if so, the // result of the conversion is a copy of the interface value X. -// If AssertedType is a superinterface of X.Type(), the operation -// cannot fail; ChangeInterface is preferred in this case. +// If AssertedType is a superinterface of X.Type(), the operation will +// fail iff the operand is nil. (Contrast with ChangeInterface, which +// performs no nil-check.) // // Type() reflects the actual type of the result, possibly a // 2-types.Tuple; AssertedType is the asserted type.