From 7072253af5300c9a2c2141cb64f57f7ff7a2282f Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 19 Aug 2013 15:38:30 -0400 Subject: [PATCH] go.tools/ssa: fixes, cleanups, cosmetic tweaks. Fix bug: the Signature for an interface method wrapper erroneously had a non-nil receiver. Function: - Set Pkg field non-nil even for wrappers. It is equal to that of the wrapped function. Only wrappers of error.Error (and its embeddings in other interfaces) may have nil. Sanity checker now asserts this. - FullName() now uses .Synthetic field to discriminate synthetic methods, not Pkg==nil. - Fullname() uses new relType() utility to print receiver type name unqualified if it belongs to the same package. (Alloc.String also uses relType utility.) CallCommon: - Description(): fix switch logic broken when we eliminated the Recv field. - better docs. R=david.crawshaw, crawshaw, gri CC=golang-dev https://golang.org/cl/13057043 --- ssa/func.go | 32 ++++++++++++++++---------------- ssa/interp/interp.go | 2 +- ssa/lvalue.go | 2 +- ssa/print.go | 34 +++++++++++++++++++++++++++++++++- ssa/promote.go | 26 ++++++++++++++++---------- ssa/sanity.go | 15 +++++++++++++-- ssa/ssa.go | 26 +++++++++++++++++++++----- 7 files changed, 101 insertions(+), 36 deletions(-) diff --git a/ssa/func.go b/ssa/func.go index ae155901..33e30da8 100644 --- a/ssa/func.go +++ b/ssa/func.go @@ -468,24 +468,24 @@ func (f *Function) fullName(from *Package) string { return f.name } - recv := f.Signature.Recv() - - // Synthetic? - if f.Pkg == nil { - var recvType types.Type - if recv != nil { - recvType = recv.Type() // promotion wrapper - } else if strings.HasPrefix(f.name, "bound$") { - return f.name // bound method wrapper - } else { - recvType = f.Params[0].Type() // interface method wrapper - } - return fmt.Sprintf("(%s).%s", recvType, f.name) + // Declared method, or promotion/indirection wrapper? + if recv := f.Signature.Recv(); recv != nil { + return fmt.Sprintf("(%s).%s", relType(recv.Type(), from), f.name) } - // Declared method? - if recv != nil { - return fmt.Sprintf("(%s).%s", recv.Type(), f.name) + // Other synthetic wrapper? + if f.Synthetic != "" { + // Bound method wrapper? + if strings.HasPrefix(f.name, "bound$") { + return f.name + } + + // Interface method wrapper? + if strings.HasPrefix(f.Synthetic, "interface ") { + return fmt.Sprintf("(%s).%s", relType(f.Params[0].Type(), from), f.name) + } + + // "package initializer" or "loaded from GC object file": fall through. } // Package-level function. diff --git a/ssa/interp/interp.go b/ssa/interp/interp.go index c318ec78..a74dd1e0 100644 --- a/ssa/interp/interp.go +++ b/ssa/interp/interp.go @@ -1,4 +1,4 @@ -// Package exp/ssa/interp defines an interpreter for the SSA +// Package ssa/interp defines an interpreter for the SSA // representation of Go programs. // // This interpreter is provided as an adjunct for testing the SSA diff --git a/ssa/lvalue.go b/ssa/lvalue.go index 3c499481..449d0bcc 100644 --- a/ssa/lvalue.go +++ b/ssa/lvalue.go @@ -91,7 +91,7 @@ func (e *element) typ() types.Type { return e.t } -// A blanks is a dummy variable whose name is "_". +// A blank is a dummy variable whose name is "_". // It is not reified: loads are illegal and stores are ignored. // type blank struct{} diff --git a/ssa/print.go b/ssa/print.go index 9c82b5d8..55d34198 100644 --- a/ssa/print.go +++ b/ssa/print.go @@ -10,6 +10,7 @@ import ( "io" "reflect" "sort" + "strings" "code.google.com/p/go.tools/go/types" ) @@ -36,6 +37,37 @@ func relName(v Value, i Instruction) string { return v.Name() } +// relType is like t.String(), but if t is a Named type belonging to +// package from, optionally wrapped by one or more Pointer +// constructors, package qualification is suppressed. +// +// TODO(gri): provide this functionality in go/types (using a +// *types.Package, obviously). +// +// TODO(adonovan): use this more widely, e.g. +// ChangeType, Literal, Convert, MakeInterface; +// when displaying receiver, params, locals, captures of a Function; +// and in the RHS type column for Value-defining Instructions. +// +func relType(t types.Type, from *Package) string { + if from != nil { + t2 := t + var nptr int // number of Pointers stripped off + for { + ptr, ok := t2.(*types.Pointer) + if !ok { + break + } + t2 = ptr.Elem() + nptr++ + } + if n, ok := t2.(*types.Named); ok && n.Obj().Pkg() == from.Object { + return strings.Repeat("*", nptr) + n.Obj().Name() + } + } + return t.String() +} + // Value.String() // // This method is provided only for debugging. @@ -77,7 +109,7 @@ func (v *Alloc) String() string { if v.Heap { op = "new" } - return fmt.Sprintf("%s %s (%s)", op, deref(v.Type()), v.Comment) + return fmt.Sprintf("%s %s (%s)", op, relType(deref(v.Type()), v.Parent().Pkg), v.Comment) } func (v *Phi) String() string { diff --git a/ssa/promote.go b/ssa/promote.go index b0501777..2d4b1d17 100644 --- a/ssa/promote.go +++ b/ssa/promote.go @@ -111,8 +111,8 @@ func findMethod(prog *Program, meth *types.Selection) *Function { // func makeWrapper(prog *Program, typ types.Type, meth *types.Selection) *Function { obj := meth.Obj().(*types.Func) - old := obj.Type().(*types.Signature) - sig := types.NewSignature(nil, types.NewVar(token.NoPos, nil, "recv", typ), old.Params(), old.Results(), old.IsVariadic()) + oldsig := obj.Type().(*types.Signature) + recv := types.NewVar(token.NoPos, nil, "recv", typ) description := fmt.Sprintf("wrapper for %s", obj) if prog.mode&LogSource != 0 { @@ -121,13 +121,14 @@ func makeWrapper(prog *Program, typ types.Type, meth *types.Selection) *Function fn := &Function{ name: obj.Name(), method: meth, - Signature: sig, + Signature: changeRecv(oldsig, recv), Synthetic: description, Prog: prog, + Pkg: prog.packages[obj.Pkg()], pos: obj.Pos(), } fn.startBody() - fn.addSpilledParam(sig.Recv()) + fn.addSpilledParam(recv) createParams(fn) var v Value = fn.Locals[0] // spilled receiver @@ -153,8 +154,8 @@ func makeWrapper(prog *Program, typ types.Type, meth *types.Selection) *Function // address of implicit C field. var c Call - if _, ok := old.Recv().Type().Underlying().(*types.Interface); !ok { // concrete method - if !isPointer(old.Recv().Type()) { + if _, ok := oldsig.Recv().Type().Underlying().(*types.Interface); !ok { // concrete method + if !isPointer(oldsig.Recv().Type()) { v = emitLoad(fn, v) } c.Call.Value = prog.declaredFunc(obj) @@ -219,17 +220,18 @@ func interfaceMethodWrapper(prog *Program, typ types.Type, obj *types.Func) *Fun // a problem, we should include 'typ' in the memoization key. fn, ok := prog.ifaceMethodWrappers[obj] if !ok { - description := fmt.Sprintf("interface method wrapper for %s.%s", typ, obj) + description := fmt.Sprintf("interface method wrapper for (%s).%s", typ, obj.Name()) if prog.mode&LogSource != 0 { defer logStack("%s", description)() } fn = &Function{ name: obj.Name(), object: obj, - Signature: obj.Type().(*types.Signature), + Signature: changeRecv(obj.Type().(*types.Signature), nil), // drop receiver Synthetic: description, pos: obj.Pos(), Prog: prog, + Pkg: prog.packages[obj.Pkg()], } fn.startBody() fn.addParam("recv", typ, token.NoPos) @@ -278,12 +280,12 @@ func boundMethodWrapper(prog *Program, obj *types.Func) *Function { if prog.mode&LogSource != 0 { defer logStack("%s", description)() } - s := obj.Type().(*types.Signature) fn = &Function{ name: "bound$" + obj.FullName(), - Signature: types.NewSignature(nil, nil, s.Params(), s.Results(), s.IsVariadic()), + Signature: changeRecv(obj.Type().(*types.Signature), nil), // drop receiver Synthetic: description, Prog: prog, + Pkg: prog.packages[obj.Pkg()], pos: obj.Pos(), } @@ -310,3 +312,7 @@ func boundMethodWrapper(prog *Program, obj *types.Func) *Function { } return fn } + +func changeRecv(s *types.Signature, recv *types.Var) *types.Signature { + return types.NewSignature(nil, recv, s.Params(), s.Results(), s.IsVariadic()) +} diff --git a/ssa/sanity.go b/ssa/sanity.go index 177d8a42..d14c031d 100644 --- a/ssa/sanity.go +++ b/ssa/sanity.go @@ -4,10 +4,12 @@ package ssa // Currently it checks CFG invariants but little at the instruction level. import ( - "code.google.com/p/go.tools/go/types" "fmt" "io" "os" + "strings" + + "code.google.com/p/go.tools/go/types" ) type sanity struct { @@ -313,7 +315,6 @@ func (s *sanity) checkBlock(b *BasicBlock, index int) { func (s *sanity) checkFunction(fn *Function) bool { // TODO(adonovan): check Function invariants: - // - check owning Package (if any) contains this (possibly anon) function // - check params match signature // - check transient fields are nil // - warn if any fn.Locals do not appear among block instructions. @@ -321,6 +322,16 @@ func (s *sanity) checkFunction(fn *Function) bool { if fn.Prog == nil { s.errorf("nil Prog") } + // All functions have a package, except wrappers for error.Error() + // (and embedding of that method in other interfaces). + if fn.Pkg == nil { + if strings.Contains(fn.Synthetic, "wrapper") && + strings.HasSuffix(fn.name, "Error") { + // wrapper for error.Error() has no package. + } else { + s.errorf("nil Pkg %q %q", fn.Synthetic, fn.name) + } + } for i, l := range fn.Locals { if l.Parent() != fn { s.errorf("Local %s at index %d has wrong parent", l.Name(), i) diff --git a/ssa/ssa.go b/ssa/ssa.go index 07e2540d..c73cc873 100644 --- a/ssa/ssa.go +++ b/ssa/ssa.go @@ -257,7 +257,7 @@ type Function struct { Synthetic string // provenance of synthetic function; "" for true source functions Enclosing *Function // enclosing function if anon; nil if global - Pkg *Package // enclosing package for Go source functions; otherwise nil + Pkg *Package // enclosing package; nil for error.Error() and its wrappers Prog *Program // enclosing program Params []*Parameter // function parameters; for methods, includes receiver FreeVars []*Capture // free variables whose values must be supplied by closure @@ -690,6 +690,9 @@ type MakeSlice struct { // The Slice instruction yields a slice of an existing string, slice // or *array X between optional integer bounds Low and High. // +// Dynamically, this instruction panics if X evaluates to a nil *array +// pointer. +// // Type() returns string if the type of X was string, otherwise a // *types.Slice with the same element type as X. // @@ -711,6 +714,9 @@ type Slice struct { // The field is identified by its index within the field list of the // struct type of X. // +// Dynamically, this instruction panics if X evaluates to a nil +// pointer. +// // Type() returns a (possibly named) *types.Pointer. // // Pos() returns the position of the ast.SelectorExpr.Sel for the @@ -749,6 +755,9 @@ type Field struct { // The elements of maps and strings are not addressable; use Lookup or // MapUpdate instead. // +// Dynamically, this instruction panics if X evaluates to a nil *array +// pointer. +// // Type() returns a (possibly named) *types.Pointer. // // Pos() returns the ast.IndexExpr.Lbrack for the index operation, if @@ -1184,7 +1193,9 @@ type anInstruction struct { // interface method invocation, or "call" and "invoke" for short. // // 1. "call" mode: when Method is nil (!IsInvoke), a CallCommon -// represents an ordinary function call of the value in Value. +// represents an ordinary function call of the value in Value, +// which may be a *Builtin, a *Function or any other value of kind +// 'func'. // // In the common case in which Value is a *Function, this indicates a // statically dispatched call to a package-level function, an @@ -1205,7 +1216,9 @@ type anInstruction struct { // 2. "invoke" mode: when Method is non-nil (IsInvoke), a CallCommon // represents a dynamically dispatched call to an interface method. // In this mode, Value is the interface value and Method is the -// interface's abstract method. +// interface's abstract method. Note: an abstract method may be +// shared by multiple interfaces due to embedding; Value.Type() +// provides the specific interface used for this call. // // Value is implicitly supplied to the concrete method implementation // as the receiver parameter; in other words, Args[0] holds not the @@ -1271,8 +1284,8 @@ func (c *CallCommon) StaticCallee() *Function { // for a user interface, e.g. "static method call". func (c *CallCommon) Description() string { switch fn := c.Value.(type) { - case nil: - return "dynamic method call" // ("invoke" mode) + case *Builtin: + return "built-in function call" case *MakeClosure: return "static function closure call" case *Function: @@ -1281,6 +1294,9 @@ func (c *CallCommon) Description() string { } return "static function call" } + if c.IsInvoke() { + return "dynamic method call" // ("invoke" mode) + } return "dynamic function call" }