From c24b2413c0bdf361c1f9d0342dcfa8dbde3bbad5 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 1 Jul 2013 15:17:36 -0400 Subject: [PATCH] go.tools/ssa: use go/types.LookupFieldOrMethod, and simplify. Added tests. R=golang-dev, gri CC=golang-dev https://golang.org/cl/10830043 --- ssa/builder.go | 196 ++++++++++++++----------------- ssa/interp/interp.go | 3 +- ssa/interp/interp_test.go | 5 +- ssa/interp/testdata/fieldprom.go | 114 ++++++++++++++++++ ssa/promote.go | 63 ---------- 5 files changed, 208 insertions(+), 173 deletions(-) create mode 100644 ssa/interp/testdata/fieldprom.go diff --git a/ssa/builder.go b/ssa/builder.go index 052cb4213f..560e5770b6 100644 --- a/ssa/builder.go +++ b/ssa/builder.go @@ -328,119 +328,89 @@ func (b *builder) builtin(fn *Function, name string, args []ast.Expr, typ types. return nil // treat all others as a regular function call } -// selector evaluates the selector expression e and returns its value, +// selectField evaluates the field selector expression e and returns its value, // or if wantAddr is true, its address, in which case escaping // indicates whether the caller intends to use the resulting pointer // in a potentially escaping way. // -func (b *builder) selector(fn *Function, e *ast.SelectorExpr, wantAddr, escaping bool) Value { - id := MakeId(e.Sel.Name, fn.Pkg.Types) +func (b *builder) selectField(fn *Function, e *ast.SelectorExpr, wantAddr, escaping bool) Value { + tx := fn.Pkg.typeOf(e.X) + obj, indices, isIndirect := types.LookupFieldOrMethod(tx, fn.Pkg.Types, e.Sel.Name) + if obj == nil { + panic("field not found: " + e.Sel.Name) + } - // Bound method closure? (e.m where m is a method) - if !wantAddr { - if m, recv := b.findMethod(fn, e.X, id); m != nil { - c := &MakeClosure{ - Fn: boundMethodWrapper(m), - Bindings: []Value{recv}, + // Be careful! This code has proven very tricky. + + // NB: The type of the final field is irrelevant to the logic. + + // Emit code for the base expression. + var v Value + if wantAddr && !isIndirect && !isPointer(tx) { + // TODO(adonovan): opt: also use this codepath + // for !wantAddr, when safe (i.e. e.X is addressible), + // since (FieldAddr;Load) is cheaper than (Load;Field). + // Requires go/types to expose addressibility. + v = b.addr(fn, e.X, escaping).(address).addr + } else { + v = b.expr(fn, e.X) + } + + // Apply field selections. + st := tx.Deref().Underlying().(*types.Struct) + for i, index := range indices { + f := st.Field(index) + ft := f.Type() + + isLast := i == len(indices)-1 + + // Invariant: v.Type() is a struct or *struct. + if isPointer(v.Type()) { + ff := &FieldAddr{ + X: v, + Field: index, } - c.setPos(e.Sel.Pos()) - c.setType(fn.Pkg.typeOf(e)) - return fn.emit(c) + if isLast { + ff.setPos(e.Sel.Pos()) + } + ff.setType(pointer(ft)) + v = fn.emit(ff) + + // Now: v is a pointer to a struct field (field lvalue). + + if isLast { + // Explicit, final field selection. + + // Load the field's value iff we don't want its address. + if !wantAddr { + v = emitLoad(fn, v) + } + } else { + // Implicit field selection. + + // Load the field's value iff indirectly embedded. + if isPointer(ft) { + v = emitLoad(fn, v) + } + } + + } else { + ff := &Field{ + X: v, + Field: index, + } + if isLast { + ff.setPos(e.Sel.Pos()) + } + ff.setType(ft) + v = fn.emit(ff) } + + // May be nil at end of last iteration: + st, _ = ft.Deref().Underlying().(*types.Struct) } - st := fn.Pkg.typeOf(e.X).Deref().Underlying().(*types.Struct) - index := -1 - for i, n := 0, st.NumFields(); i < n; i++ { - f := st.Field(i) - if MakeId(f.Name(), f.Pkg()) == id { - index = i - break - } - } - var path *anonFieldPath - if index == -1 { - // Not a named field. Use breadth-first algorithm. - path, index = findPromotedField(st, id) - if path == nil { - panic("field not found, even with promotion: " + e.Sel.Name) - } - } - fieldType := fn.Pkg.typeOf(e) - pos := e.Sel.Pos() - if wantAddr { - return b.fieldAddr(fn, e.X, path, index, fieldType, pos, escaping) - } - return b.fieldExpr(fn, e.X, path, index, fieldType, pos) -} - -// fieldAddr evaluates the base expression (a struct or *struct), -// applies to it any implicit field selections from path, and then -// selects the field #index of type fieldType. -// Its address is returned. -// -// (fieldType can be derived from base+index.) -// -func (b *builder) fieldAddr(fn *Function, base ast.Expr, path *anonFieldPath, index int, fieldType types.Type, pos token.Pos, escaping bool) Value { - var x Value - if path != nil { - switch path.field.Type().Underlying().(type) { - case *types.Struct: - x = b.fieldAddr(fn, base, path.tail, path.index, path.field.Type(), token.NoPos, escaping) - case *types.Pointer: - x = b.fieldExpr(fn, base, path.tail, path.index, path.field.Type(), token.NoPos) - } - } else { - switch fn.Pkg.typeOf(base).Underlying().(type) { - case *types.Struct: - x = b.addr(fn, base, escaping).(address).addr - case *types.Pointer: - x = b.expr(fn, base) - } - } - v := &FieldAddr{ - X: x, - Field: index, - } - v.setPos(pos) - v.setType(pointer(fieldType)) - return fn.emit(v) -} - -// fieldExpr evaluates the base expression (a struct or *struct), -// applies to it any implicit field selections from path, and then -// selects the field #index of type fieldType. -// Its value is returned. -// -// (fieldType can be derived from base+index.) -// -func (b *builder) fieldExpr(fn *Function, base ast.Expr, path *anonFieldPath, index int, fieldType types.Type, pos token.Pos) Value { - var x Value - if path != nil { - x = b.fieldExpr(fn, base, path.tail, path.index, path.field.Type(), token.NoPos) - } else { - x = b.expr(fn, base) - } - switch x.Type().Underlying().(type) { - case *types.Struct: - v := &Field{ - X: x, - Field: index, - } - v.setPos(pos) - v.setType(fieldType) - return fn.emit(v) - - case *types.Pointer: // *struct - v := &FieldAddr{ - X: x, - Field: index, - } - v.setPos(pos) - v.setType(pointer(fieldType)) - return emitLoad(fn, fn.emit(v)) - } - panic("unreachable") + return v } // addr lowers a single-result addressable expression e to SSA form, @@ -500,7 +470,7 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue { } // e.f where e is an expression. - return address{addr: b.selector(fn, e, true, escaping)} + return address{addr: b.selectField(fn, e, true, escaping)} case *ast.IndexExpr: var x Value @@ -730,9 +700,10 @@ func (b *builder) expr(fn *Function, e ast.Expr) Value { return b.expr(fn, e.Sel) } + id := MakeId(e.Sel.Name, fn.Pkg.Types) + // (*T).f or T.f, the method f from the method-set of type T. if fn.Pkg.info.IsType(e.X) { - id := MakeId(e.Sel.Name, fn.Pkg.Types) typ := fn.Pkg.typeOf(e.X) if m := fn.Prog.MethodSet(typ)[id]; m != nil { return emitConv(fn, m, fn.Pkg.typeOf(e)) @@ -742,8 +713,19 @@ func (b *builder) expr(fn *Function, e ast.Expr) Value { return interfaceMethodWrapper(fn.Prog, typ, id) } + // Bound method closure? (e.m where m is a method) + if m, recv := b.findMethod(fn, e.X, id); m != nil { + c := &MakeClosure{ + Fn: boundMethodWrapper(m), + Bindings: []Value{recv}, + } + c.setPos(e.Sel.Pos()) + c.setType(fn.Pkg.typeOf(e)) + return fn.emit(c) + } + // e.f where e is an expression. f may be a method. - return b.selector(fn, e, false, false) + return b.selectField(fn, e, false, false) case *ast.IndexExpr: switch t := fn.Pkg.typeOf(e.X).Underlying().(type) { diff --git a/ssa/interp/interp.go b/ssa/interp/interp.go index 7f0308352a..d8f8a8cc59 100644 --- a/ssa/interp/interp.go +++ b/ssa/interp/interp.go @@ -480,7 +480,8 @@ func callSSA(i *interpreter, caller *frame, callpos token.Pos, fn *ssa.Function, if fr.i.mode&DisableRecover != 0 { return // let interpreter crash } - fr.status, fr.panic = stPanic, recover() + fr.status = stPanic + fr.panic = recover() } fr.rundefers() // Destroy the locals to avoid accidental use after return. diff --git a/ssa/interp/interp_test.go b/ssa/interp/interp_test.go index 2fd7c524b1..7befccaf15 100644 --- a/ssa/interp/interp_test.go +++ b/ssa/interp/interp_test.go @@ -133,6 +133,7 @@ var testdataTests = []string{ "mrvchain.go", "boundmeth.go", "ifaceprom.go", + "fieldprom.go", } func run(t *testing.T, dir, input string) bool { @@ -164,7 +165,7 @@ func run(t *testing.T, dir, input string) bool { } }() - hint = fmt.Sprintf("To dump SSA representation, run:\n%% go run exp/ssa/ssadump.go -build=CFP %s\n", input) + hint = fmt.Sprintf("To dump SSA representation, run:\n%% go run src/code.google.com/p/go.tools/ssa/ssadump.go -build=CFP %s\n", input) info, err := imp.CreateSourcePackage("main", files) if err != nil { t.Errorf("ssa.Builder.CreatePackage(%s) failed: %s", inputs, err.Error()) @@ -175,7 +176,7 @@ func run(t *testing.T, dir, input string) bool { prog.CreatePackages(imp) prog.BuildAll() - hint = fmt.Sprintf("To trace execution, run:\n%% go run exp/ssa/ssadump.go -build=C -run --interp=T %s\n", input) + hint = fmt.Sprintf("To trace execution, run:\n%% go run src/code.google.com/p/go.tools/ssa/ssadump.go -build=C -run --interp=T %s\n", input) if exitCode := interp.Interpret(prog.Package(info.Pkg), 0, inputs[0], []string{}); exitCode != 0 { t.Errorf("interp.Interpret(%s) exited with code %d, want zero", inputs, exitCode) return false diff --git a/ssa/interp/testdata/fieldprom.go b/ssa/interp/testdata/fieldprom.go new file mode 100644 index 0000000000..fc276ddbf0 --- /dev/null +++ b/ssa/interp/testdata/fieldprom.go @@ -0,0 +1,114 @@ +package main + +// Tests of field promotion logic. + +type A struct { + x int + y *int +} + +type B struct { + p int + q *int +} + +type C struct { + A + *B +} + +type D struct { + a int + C +} + +func assert(cond bool) { + if !cond { + panic("failed") + } +} + +func f1(c C) { + assert(c.x == c.A.x) + assert(c.y == c.A.y) + assert(&c.x == &c.A.x) + assert(&c.y == &c.A.y) + + assert(c.p == c.B.p) + assert(c.q == c.B.q) + assert(&c.p == &c.B.p) + assert(&c.q == &c.B.q) + + c.x = 1 + *c.y = 1 + c.p = 1 + *c.q = 1 +} + +func f2(c *C) { + assert(c.x == c.A.x) + assert(c.y == c.A.y) + assert(&c.x == &c.A.x) + assert(&c.y == &c.A.y) + + assert(c.p == c.B.p) + assert(c.q == c.B.q) + assert(&c.p == &c.B.p) + assert(&c.q == &c.B.q) + + c.x = 1 + *c.y = 1 + c.p = 1 + *c.q = 1 +} + +func f3(d D) { + assert(d.x == d.C.A.x) + assert(d.y == d.C.A.y) + assert(&d.x == &d.C.A.x) + assert(&d.y == &d.C.A.y) + + assert(d.p == d.C.B.p) + assert(d.q == d.C.B.q) + assert(&d.p == &d.C.B.p) + assert(&d.q == &d.C.B.q) + + d.x = 1 + *d.y = 1 + d.p = 1 + *d.q = 1 +} + +func f4(d *D) { + assert(d.x == d.C.A.x) + assert(d.y == d.C.A.y) + assert(&d.x == &d.C.A.x) + assert(&d.y == &d.C.A.y) + + assert(d.p == d.C.B.p) + assert(d.q == d.C.B.q) + assert(&d.p == &d.C.B.p) + assert(&d.q == &d.C.B.q) + + d.x = 1 + *d.y = 1 + d.p = 1 + *d.q = 1 +} + +func main() { + y := 123 + c := C{ + A{x: 42, y: &y}, + &B{p: 42, q: &y}, + } + + assert(&c.x == &c.A.x) + + f1(c) + f2(&c) + + d := D{C: c} + f3(d) + f4(&d) +} diff --git a/ssa/promote.go b/ssa/promote.go index 07978d2501..d1ec9e9102 100644 --- a/ssa/promote.go +++ b/ssa/promote.go @@ -512,66 +512,3 @@ func indirectionWrapper(meth *Function) *Function { } return fn } - -// Implicit field promotion ---------------------------------------- - -// For a given struct type and (promoted) field Id, findEmbeddedField -// returns the path of implicit anonymous field selections, and the -// field index of the explicit (=outermost) selection. -// -// TODO(gri): if go/types/operand.go's lookupFieldBreadthFirst were to -// record (e.g. call a client-provided callback) the implicit field -// selection path discovered for a particular ast.SelectorExpr, we could -// eliminate this function. -// -func findPromotedField(st *types.Struct, id Id) (*anonFieldPath, int) { - // visited records the types that have been searched already. - // Invariant: keys are all *types.Named. - // (types.Type is not a sound map key in general.) - visited := make(map[types.Type]bool) - - var list, next []*anonFieldPath - for i, n := 0, st.NumFields(); i < n; i++ { - f := st.Field(i) - if f.Anonymous() { - list = append(list, &anonFieldPath{nil, i, f}) - } - } - - // Search the current level if there is any work to do and collect - // embedded types of the next lower level in the next list. - for { - // look for name in all types at this level - for _, node := range list { - typ := node.field.Type().Deref().(*types.Named) - if visited[typ] { - continue - } - visited[typ] = true - - switch typ := typ.Underlying().(type) { - case *types.Struct: - for i, n := 0, typ.NumFields(); i < n; i++ { - f := typ.Field(i) - if MakeId(f.Name(), f.Pkg()) == id { - return node, i - } - } - for i, n := 0, typ.NumFields(); i < n; i++ { - f := typ.Field(i) - if f.Anonymous() { - next = append(next, &anonFieldPath{node, i, f}) - } - } - } - } - - if len(next) == 0 { - panic("field not found: " + id.String()) - } - - // No match so far. - list, next = next, list[:0] // reuse arrays - } - panic("unreachable") -}