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

@@ -468,24 +468,24 @@ func (f *Function) fullName(from *Package) string {
 		return f.name
 	}
 
-	recv := f.Signature.Recv()
+	// Declared method, or promotion/indirection wrapper?
-
+	if recv := f.Signature.Recv(); recv != nil {
-	// Synthetic?
+		return fmt.Sprintf("(%s).%s", relType(recv.Type(), from), f.name)
-	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?
+	// Other synthetic wrapper?
-	if recv != nil {
+	if f.Synthetic != "" {
-		return fmt.Sprintf("(%s).%s", recv.Type(), f.name)
+		// 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.

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

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{}

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 {

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)
+	oldsig := obj.Type().(*types.Signature)
-	sig := types.NewSignature(nil, types.NewVar(token.NoPos, nil, "recv", typ), old.Params(), old.Results(), old.IsVariadic())
+	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 _, ok := oldsig.Recv().Type().Underlying().(*types.Interface); !ok { // concrete method
-		if !isPointer(old.Recv().Type()) {
+		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())
+}

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)

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:
+	case *Builtin:
-		return "dynamic method call" // ("invoke" mode)
+		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"
 }