go.tools/ssa: several small clean-ups.

- removed a number of obsolete TODO(gri) comments.
- bring ssa.DefaultType back into sync with types.defaultType.
- re-enable types.Package.Path()!="" assertion.
- use Path() (not reflect pointer) in sort routine.
- make interp.checkInterface use types.MissingMethod.
- un-export ssa.MakeId function.
- inline pointer() into all callers, and delete.
- enable two more interp_tests: $GOROOT/test/{method3,cmp}.go
- add links to bugs to other interp_tests.
- add runtime.NumCPU to ssa/interp/externals.go

R=gri
CC=golang-dev
https://golang.org/cl/11353043

go/types/predicates.go

@@ -238,7 +238,7 @@ func identicalMethods(a, b []*Func) bool {
 func defaultType(typ Type) Type {
 	if t, ok := typ.(*Basic); ok {
 		k := t.kind
-		switch t.kind {
+		switch k {
 		case UntypedBool:
 			k = Bool
 		case UntypedInt:

ssa/builder.go

@@ -373,7 +373,7 @@ func (b *builder) selectField(fn *Function, e *ast.SelectorExpr, wantAddr, escap
 			if isLast {
 				ff.setPos(e.Sel.Pos())
 			}
-			ff.setType(pointer(ft))
+			ff.setType(types.NewPointer(ft))
 			v = fn.emit(ff)
 
 			// Now: v is a pointer to a struct field (field lvalue).
@@ -481,13 +481,13 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
 		switch t := fn.Pkg.typeOf(e.X).Underlying().(type) {
 		case *types.Array:
 			x = b.addr(fn, e.X, escaping).address(fn)
-			et = pointer(t.Elem())
+			et = types.NewPointer(t.Elem())
 		case *types.Pointer: // *array
 			x = b.expr(fn, e.X)
-			et = pointer(t.Elem().Underlying().(*types.Array).Elem())
+			et = types.NewPointer(t.Elem().Underlying().(*types.Array).Elem())
 		case *types.Slice:
 			x = b.expr(fn, e.X)
-			et = pointer(t.Elem())
+			et = types.NewPointer(t.Elem())
 		case *types.Map:
 			return &element{
 				m: b.expr(fn, e.X),
@@ -708,7 +708,7 @@ func (b *builder) expr(fn *Function, e ast.Expr) Value {
 			return b.expr(fn, e.Sel)
 		}
 
-		id := MakeId(e.Sel.Name, fn.Pkg.Object)
+		id := makeId(e.Sel.Name, fn.Pkg.Object)
 
 		// (*T).f or T.f, the method f from the method-set of type T.
 		if fn.Pkg.info.IsType(e.X) {
@@ -814,7 +814,7 @@ func (b *builder) findMethod(fn *Function, base ast.Expr, id Id) (*Function, Val
 	}
 	if !isPointer(typ) {
 		// Consult method-set of *X.
-		if m := fn.Prog.MethodSet(pointer(typ))[id]; m != nil {
+		if m := fn.Prog.MethodSet(types.NewPointer(typ))[id]; m != nil {
 			// A method found only in MS(*X) must have a
 			// pointer formal receiver; but the actual
 			// value is not a pointer.
@@ -852,7 +852,7 @@ func (b *builder) setCallFunc(fn *Function, e *ast.CallExpr, c *CallCommon) {
 		return
 	}
 
-	id := MakeId(sel.Sel.Name, fn.Pkg.Object)
+	id := makeId(sel.Sel.Name, fn.Pkg.Object)
 
 	// Let X be the type of x.
 
@@ -944,7 +944,7 @@ func (b *builder) emitCallArgs(fn *Function, sig *types.Signature, e *ast.CallEx
 					X:     a,
 					Index: intLiteral(int64(i)),
 				}
-				iaddr.setType(pointer(vt))
+				iaddr.setType(types.NewPointer(vt))
 				fn.emit(iaddr)
 				emitStore(fn, iaddr, arg)
 			}
@@ -1042,7 +1042,7 @@ func (b *builder) globalValueSpec(init *Function, spec *ast.ValueSpec, g *Global
 		for i, id := range spec.Names {
 			var lval lvalue = blank{}
 			if g != nil {
-				// Mode A: initialized only a single global, g
+				// Mode A: initialize only a single global, g
 				if isBlankIdent(id) || init.Pkg.objectOf(id) != obj {
 					continue
 				}
@@ -1241,7 +1241,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, typ typ
 				X:     addr,
 				Field: fieldIndex,
 			}
-			faddr.setType(pointer(sf.Type()))
+			faddr.setType(types.NewPointer(sf.Type()))
 			fn.emit(faddr)
 			b.exprInPlace(fn, address{addr: faddr}, e)
 		}
@@ -1274,7 +1274,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, typ typ
 				X:     array,
 				Index: idx,
 			}
-			iaddr.setType(pointer(at.Elem()))
+			iaddr.setType(types.NewPointer(at.Elem()))
 			fn.emit(iaddr)
 			b.exprInPlace(fn, address{addr: iaddr}, e)
 		}
@@ -1489,7 +1489,7 @@ func (b *builder) typeSwitchStmt(fn *Function, s *ast.TypeSwitchStmt, label *lbl
 			// same name but a more specific type.
 			y2 := fn.addNamedLocal(obj)
 			y2.name += "'" // debugging aid
-			y2.typ = pointer(casetype)
+			y2.typ = types.NewPointer(casetype)
 			emitStore(fn, y2, ti)
 		}
 		fn.targets = &targets{
@@ -1787,7 +1787,7 @@ func (b *builder) rangeIndexed(fn *Function, x Value, tv types.Type) (k, v Value
 				X:     x,
 				Index: k,
 			}
-			instr.setType(pointer(t.Elem().(*types.Array).Elem()))
+			instr.setType(types.NewPointer(t.Elem().(*types.Array).Elem()))
 			v = emitLoad(fn, fn.emit(instr))
 
 		case *types.Slice:
@@ -1795,7 +1795,7 @@ func (b *builder) rangeIndexed(fn *Function, x Value, tv types.Type) (k, v Value
 				X:     x,
 				Index: k,
 			}
-			instr.setType(pointer(t.Elem()))
+			instr.setType(types.NewPointer(t.Elem()))
 			v = emitLoad(fn, fn.emit(instr))
 
 		default:
@@ -2265,6 +2265,9 @@ func (b *builder) buildDecl(pkg *Package, decl ast.Decl) {
 		if isBlankIdent(id) {
 			// no-op
 
+			// TODO(adonovan): test: can references within
+			// the blank functions' body affect the program?
+
 		} else if id.Name == "init" {
 			// init() block
 			if pkg.Prog.mode&LogSource != 0 {

ssa/create.go

@@ -102,7 +102,7 @@ func memberFromObject(pkg *Package, obj types.Object, syntax ast.Node) {
 			Pkg:    pkg,
 			name:   name,
 			object: obj,
-			typ:    pointer(obj.Type()), // address
+			typ:    types.NewPointer(obj.Type()), // address
 			pos:    obj.Pos(),
 			spec:   spec,
 		}
@@ -139,7 +139,7 @@ func memberFromObject(pkg *Package, obj types.Object, syntax ast.Node) {
 			// Method declaration.
 			_, method := namedTypeMethodIndex(
 				deref(recv.Type()).(*types.Named),
-				MakeId(name, pkg.Object))
+				makeId(name, pkg.Object))
 			pkg.Prog.concreteMethods[method] = fn
 		}
 
@@ -257,7 +257,7 @@ func createPackage(prog *Program, importPath string, info *importer.PackageInfo)
 	initguard := &Global{
 		Pkg:  p,
 		name: "init$guard",
-		typ:  pointer(tBool),
+		typ:  types.NewPointer(tBool),
 	}
 	p.Members[initguard.Name()] = initguard
 

ssa/emit.go

@@ -14,7 +14,7 @@ import (
 //
 func emitNew(f *Function, typ types.Type, pos token.Pos) Value {
 	return f.emit(&Alloc{
-		typ:  pointer(typ),
+		typ:  types.NewPointer(typ),
 		Heap: true,
 		pos:  pos,
 	})
@@ -194,7 +194,6 @@ func emitConv(f *Function, val Value, typ types.Type) Value {
 		}
 
 		// Convert (non-nil) "untyped" literals to their default type.
-		// TODO(gri): expose types.isUntyped().
 		if t, ok := ut_src.(*types.Basic); ok && t.Info()&types.IsUntyped != 0 {
 			val = emitConv(f, val, DefaultType(ut_src))
 		}

ssa/func.go

@@ -197,7 +197,7 @@ func (f *Function) addSpilledParam(obj types.Object) {
 	param := f.addParamObj(obj)
 	spill := &Alloc{
 		name: obj.Name() + "~", // "~" means "spilled"
-		typ:  pointer(obj.Type()),
+		typ:  types.NewPointer(obj.Type()),
 		pos:  obj.Pos(),
 	}
 	f.objects[obj] = spill
@@ -401,7 +401,7 @@ func (f *Function) addLocalForIdent(id *ast.Ident) *Alloc {
 // to function f and returns it.  pos is the optional source location.
 //
 func (f *Function) addLocal(typ types.Type, pos token.Pos) *Alloc {
-	v := &Alloc{typ: pointer(typ), pos: pos}
+	v := &Alloc{typ: types.NewPointer(typ), pos: pos}
 	f.Locals = append(f.Locals, v)
 	f.emit(v)
 	return v

ssa/interp/external.go

@@ -63,6 +63,7 @@ var externals = map[string]externalFn{
 	"runtime.GC":                      ext۰runtime۰GC,
 	"runtime.GOMAXPROCS":              ext۰runtime۰GOMAXPROCS,
 	"runtime.Gosched":                 ext۰runtime۰Gosched,
+	"runtime.NumCPU":                  ext۰runtime۰NumCPU,
 	"runtime.ReadMemStats":            ext۰runtime۰ReadMemStats,
 	"runtime.SetFinalizer":            ext۰runtime۰SetFinalizer,
 	"runtime.getgoroot":               ext۰runtime۰getgoroot,
@@ -163,6 +164,10 @@ func ext۰runtime۰Gosched(fn *ssa.Function, args []value) value {
 	return nil
 }
 
+func ext۰runtime۰NumCPU(fn *ssa.Function, args []value) value {
+	return runtime.NumCPU()
+}
+
 func ext۰runtime۰ReadMemStats(fn *ssa.Function, args []value) value {
 	// TODO(adonovan): populate args[0].(Struct)
 	return nil
@@ -289,7 +294,6 @@ func ext۰syscall۰Getpid(fn *ssa.Function, args []value) value {
 // os/signal/signal_unix.go:16:func signal_recv() uint32
 // runtime/debug.go:13:func LockOSThread()
 // runtime/debug.go:17:func UnlockOSThread()
-// runtime/debug.go:27:func NumCPU() int
 // runtime/debug.go:30:func NumCgoCall() int64
 // runtime/debug.go:33:func NumGoroutine() int
 // runtime/debug.go:90:func MemProfile(p []MemProfileRecord, inuseZero bool) (n int, ok bool)

ssa/interp/interp_test.go

@@ -33,6 +33,7 @@ var gorootTests = []string{
 	"varinit.go",
 	"escape3.go",
 	"initcomma.go",
+	"cmp.go",
 	"compos.go",
 	"turing.go",
 	"indirect.go",
@@ -68,6 +69,7 @@ var gorootTests = []string{
 	"typeswitch.go",
 	"stringrange.go",
 	"reorder.go",
+	"method3.go",
 	"literal.go",
 	"nul1.go",
 	"zerodivide.go",
@@ -100,17 +102,16 @@ var gorootTests = []string{
 	// "solitaire.go", // works, but too slow (~30s).
 	// "const.go",     // works but for but one bug: constant folder doesn't consider representations.
 	// "init1.go",     // too slow (80s) and not that interesting. Cheats on ReadMemStats check too.
+	// "rotate.go rotate0.go", // emits source for a test
+	// "rotate.go rotate1.go", // emits source for a test
+	// "rotate.go rotate2.go", // emits source for a test
+	// "rotate.go rotate3.go", // emits source for a test
+	// "64bit.go",             // emits source for a test
+	// "run.go",               // test driver, not a test.
 
 	// Typechecker failures:
-	// "switch.go",            // bug re: switch ... { case 1.0:... case 1:... }
+	// "switch.go",            // https://code.google.com/p/go/issues/detail?id=5505
-	// "rune.go",              // error re: rune as index
+	// "rune.go",              // https://code.google.com/p/go/issues/detail?id=5895
-	// "64bit.go",             // error re: comparison
-	// "cmp.go",               // error re: comparison
-	// "rotate.go rotate0.go", // error re: shifts
-	// "rotate.go rotate1.go", // error re: shifts
-	// "rotate.go rotate2.go", // error re: shifts
-	// "rotate.go rotate3.go", // error re: shifts
-	// "run.go",               // produces wrong constant for bufio.runeError; also, not really a test.
 
 	// Broken.  TODO(adonovan): fix.
 	// copy.go         // very slow; but with N=4 quickly crashes, slice index out of range.
@@ -118,7 +119,6 @@ var gorootTests = []string{
 	// recover1.go     // error: "spurious recover"
 	// recover2.go     // panic: interface conversion: string is not error: missing method Error
 	// recover3.go     // logic errors: panicked with wrong Error.
-	// method3.go      // Fails dynamically; (*T).f vs (T).f are distinct methods.
 	// args.go         // works, but requires specific os.Args from the driver.
 	// index.go        // a template, not a real test.
 	// mallocfin.go    // SetFinalizer not implemented.

ssa/interp/ops.go

@@ -1324,14 +1324,13 @@ func conv(t_dst, t_src types.Type, x value) value {
 // On success it returns "", on failure, an error message.
 //
 func checkInterface(i *interpreter, itype *types.Interface, x iface) string {
-	mset := findMethodSet(i, x.t)
+	if meth, wrongType := types.MissingMethod(x.t, itype); meth != nil {
-	it := itype.Underlying().(*types.Interface)
+		reason := "is missing"
-	for i, n := 0, it.NumMethods(); i < n; i++ {
+		if wrongType {
-		m := it.Method(i)
+			reason = "has wrong type"
-		id := ssa.MakeId(m.Name(), m.Pkg())
-		if mset[id] == nil {
-			return fmt.Sprintf("interface conversion: %v is not %v: missing method %v", x.t, itype, id)
 		}
+		return fmt.Sprintf("interface conversion: %v is not %v: method %s %s",
+			x.t, itype, meth.Name(), reason)
 	}
 	return "" // ok
 }

ssa/print.go

@@ -392,7 +392,7 @@ func (p *Package) DumpTo(w io.Writer) {
 			// methods themselves may differ,
 			// e.g. promotion wrappers.
 			// NB: if mem.Type() is a pointer, mset is empty.
-			mset := p.Prog.MethodSet(pointer(mem.Type()))
+			mset := p.Prog.MethodSet(types.NewPointer(mem.Type()))
 			var keys ids
 			for id := range mset {
 				keys = append(keys, id)

ssa/promote.go

@@ -176,7 +176,7 @@ func buildMethodSet(prog *Program, typ types.Type) MethodSet {
 			case *types.Struct:
 				for i, n := 0, t.NumFields(); i < n; i++ {
 					f := t.Field(i)
-					nextcands[MakeId(f.Name(), f.Pkg())] = nil // a field: block id
+					nextcands[makeId(f.Name(), f.Pkg())] = nil // a field: block id
 					// Queue up anonymous fields for next iteration.
 					// Break cycles to ensure termination.
 					if f.Anonymous() && !node.contains(f) {
@@ -244,7 +244,7 @@ func buildMethodSet(prog *Program, typ types.Type) MethodSet {
 // If a map entry already exists (whether nil or not), its value is set to nil.
 //
 func addCandidate(m map[Id]*candidate, method *types.Func, node *anonFieldPath) {
-	id := MakeId(method.Name(), method.Pkg())
+	id := makeId(method.Name(), method.Pkg())
 	prev, found := m[id]
 	switch {
 	case prev != nil:
@@ -318,7 +318,7 @@ func promotionWrapper(prog *Program, typ types.Type, cand *candidate) *Function
 			X:     v,
 			Field: p.index,
 		}
-		sel.setType(pointer(p.field.Type()))
+		sel.setType(types.NewPointer(p.field.Type()))
 		v = fn.emit(sel)
 		if isPointer(p.field.Type()) {
 			v = emitLoad(fn, v)
@@ -334,7 +334,7 @@ func promotionWrapper(prog *Program, typ types.Type, cand *candidate) *Function
 		c.Call.Args = append(c.Call.Args, v)
 	} else {
 		iface := v.Type().Underlying().(*types.Interface)
-		id := MakeId(cand.method.Name(), cand.method.Pkg())
+		id := makeId(cand.method.Name(), cand.method.Pkg())
 		c.Call.Method, _ = interfaceMethodIndex(iface, id)
 		c.Call.Recv = v
 	}

ssa/source.go

@@ -105,7 +105,7 @@ func findNamedFunc(pkg *Package, pos token.Pos) *Function {
 					return meth
 				}
 			}
-			for _, meth := range pkg.Prog.MethodSet(pointer(mem.Type())) {
+			for _, meth := range pkg.Prog.MethodSet(types.NewPointer(mem.Type())) {
 				if meth.Synthetic == "" && meth.Pos() == pos {
 					return meth
 				}

ssa/ssa.go

@@ -1286,7 +1286,7 @@ func (c *CallCommon) StaticCallee() *Function {
 // have "invoke" mode.
 func (c *CallCommon) MethodId() Id {
 	m := c.Recv.Type().Underlying().(*types.Interface).Method(c.Method)
-	return MakeId(m.Name(), m.Pkg())
+	return makeId(m.Name(), m.Pkg())
 }
 
 // Description returns a description of the mode of this call suitable

ssa/util.go

@@ -7,7 +7,6 @@ import (
 	"go/ast"
 	"io"
 	"os"
-	"reflect"
 
 	"code.google.com/p/go.tools/go/types"
 )
@@ -33,9 +32,6 @@ func unparen(e ast.Expr) ast.Expr {
 // isBlankIdent returns true iff e is an Ident with name "_".
 // They have no associated types.Object, and thus no type.
 //
-// TODO(gri): consider making typechecker not treat them differently.
-// It's one less thing for clients like us to worry about.
-//
 func isBlankIdent(e ast.Expr) bool {
 	id, ok := e.(*ast.Ident)
 	return ok && id.Name == "_"
@@ -49,12 +45,6 @@ func isPointer(typ types.Type) bool {
 	return ok
 }
 
-// pointer(typ) returns the type that is a pointer to typ.
-// TODO(adonovan): inline and eliminate.
-func pointer(typ types.Type) *types.Pointer {
-	return types.NewPointer(typ)
-}
-
 // deref returns a pointer's element type; otherwise it returns typ.
 func deref(typ types.Type) types.Type {
 	if p, ok := typ.Underlying().(*types.Pointer); ok {
@@ -67,12 +57,10 @@ func deref(typ types.Type) types.Type {
 // within the set of explicitly declared concrete methods of named
 // type typ.  If not found, panic ensues.
 //
-// TODO(gri): move this functionality into the go/types API?
-//
 func namedTypeMethodIndex(typ *types.Named, id Id) (int, *types.Func) {
 	for i, n := 0, typ.NumMethods(); i < n; i++ {
 		m := typ.Method(i)
-		if MakeId(m.Name(), m.Pkg()) == id {
+		if makeId(m.Name(), m.Pkg()) == id {
 			return i, m
 		}
 	}
@@ -83,12 +71,10 @@ func namedTypeMethodIndex(typ *types.Named, id Id) (int, *types.Func) {
 // within the method-set of interface type typ.  If not found, panic
 // ensues.
 //
-// TODO(gri): move this functionality into the go/types API.
-//
 func interfaceMethodIndex(typ *types.Interface, id Id) (int, *types.Func) {
 	for i, n := 0, typ.NumMethods(); i < n; i++ {
 		m := typ.Method(i)
-		if MakeId(m.Name(), m.Pkg()) == id {
+		if makeId(m.Name(), m.Pkg()) == id {
 			return i, m
 		}
 	}
@@ -108,7 +94,7 @@ outer:
 		xm := x.Method(i)
 		for j, m := 0, y.NumMethods(); j < m; j++ {
 			ym := y.Method(j)
-			if MakeId(xm.Name(), xm.Pkg()) == MakeId(ym.Name(), ym.Pkg()) {
+			if makeId(xm.Name(), xm.Pkg()) == makeId(ym.Name(), ym.Pkg()) {
 				if !types.IsIdentical(xm.Type(), ym.Type()) {
 					return false // common name but conflicting types
 				}
@@ -141,8 +127,8 @@ func canHaveConcreteMethods(typ types.Type, allowPtr bool) bool {
 }
 
 // DefaultType returns the default "typed" type for an "untyped" type;
-// it returns the incoming type for all other types. If there is no
+// it returns the incoming type for all other types.  The default type
-// corresponding untyped type, the result is types.Typ[types.Invalid].
+// for untyped nil is untyped nil.
 //
 // Exported to exp/ssa/interp.
 //
@@ -150,11 +136,8 @@ func canHaveConcreteMethods(typ types.Type, allowPtr bool) bool {
 //
 func DefaultType(typ types.Type) types.Type {
 	if t, ok := typ.(*types.Basic); ok {
-		k := types.Invalid
+		k := t.Kind()
-		switch t.Kind() {
+		switch k {
-		// case UntypedNil:
-		//      There is no default type for nil. For a good error message,
-		//      catch this case before calling this function.
 		case types.UntypedBool:
 			k = types.Bool
 		case types.UntypedInt:
@@ -176,16 +159,13 @@ func DefaultType(typ types.Type) types.Type {
 // makeId returns the Id (name, pkg) if the name is exported or
 // (name, nil) otherwise.
 //
-// Exported to exp/ssa/interp.
+func makeId(name string, pkg *types.Package) (id Id) {
-//
-func MakeId(name string, pkg *types.Package) (id Id) {
 	id.Name = name
 	if !ast.IsExported(name) {
 		id.Pkg = pkg
-		// TODO(gri): fix
+		if pkg.Path() == "" {
-		// if pkg.Path() == "" {
+			panic("empty types.Package.Path()")
-		// 	panic("Package " + pkg.Name() + "has empty Path")
+		}
-		// }
 	}
 	return
 }
@@ -195,11 +175,9 @@ type ids []Id // a sortable slice of Id
 func (p ids) Len() int { return len(p) }
 func (p ids) Less(i, j int) bool {
 	x, y := p[i], p[j]
-	// *Package pointers are canonical so order by them.
+	// TODO(adonovan): opt: where's Go's 3-valued strings.Compare function?
-	// Don't use x.Pkg.ImportPath because sometimes it's empty.
+	return x.Pkg.Path() < y.Pkg.Path() ||
-	// (TODO(gri): fix that.)
+		x.Pkg.Path() == y.Pkg.Path() && x.Name < y.Name
-	return reflect.ValueOf(x.Pkg).Pointer() < reflect.ValueOf(y.Pkg).Pointer() ||
-		x.Pkg == y.Pkg && x.Name < y.Name
 }
 func (p ids) Swap(i, j int) { p[i], p[j] = p[j], p[i] }