go/ssa: consolidate use of underlying pointer

Consolidates the use of typ.Underlying().(*types.Pointer)
to the deptr function. This function replace isPointer and the
old deref function. Both have been replaced. This allows for
tracking where underlying pointers are used. Follow up CLs
will try to move away from using underlying pointers when necessary.

Switches deptr for mustDeref in obviously safe locations (alloc.Type()).

Adds a new deref function that uses the core type
instead of the underlying type.

Change-Id: Id51f95e87cb40a13d43fd595d1f20b21b8325eeb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/494976
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Tim King <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: timothy-king

go/ssa/builder.go

@@ -363,7 +363,7 @@ func (b *builder) builtin(fn *Function, obj *types.Builtin, args []ast.Expr, typ
 		}
 
 	case "new":
-		alloc := emitNew(fn, deref(typ), pos)
+		alloc := emitNew(fn, mustDeref(typ), pos)
 		alloc.Comment = "new"
 		return alloc
 
@@ -375,8 +375,8 @@ func (b *builder) builtin(fn *Function, obj *types.Builtin, args []ast.Expr, typ
 		// been constant-folded.)
 		//
 		// Type parameters are always non-constant so use Underlying.
-		t := deref(fn.typeOf(args[0])).Underlying()
-		if at, ok := t.(*types.Array); ok {
+		t, _ := deptr(fn.typeOf(args[0]))
+		if at, ok := t.Underlying().(*types.Array); ok {
 			b.expr(fn, args[0]) // for effects only
 			return intConst(at.Len())
 		}
@@ -431,7 +431,7 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
 		return &address{addr: v, pos: e.Pos(), expr: e}
 
 	case *ast.CompositeLit:
-		t := deref(fn.typeOf(e))
+		t, _ := deptr(fn.typeOf(e))
 		var v *Alloc
 		if escaping {
 			v = emitNew(fn, t, e.Lbrace)
@@ -459,7 +459,8 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
 		wantAddr := true
 		v := b.receiver(fn, e.X, wantAddr, escaping, sel)
 		index := sel.index[len(sel.index)-1]
-		fld := typeparams.CoreType(deref(v.Type())).(*types.Struct).Field(index)
+		dt, _ := deptr(v.Type())
+		fld := typeparams.CoreType(dt).(*types.Struct).Field(index)
 
 		// Due to the two phases of resolving AssignStmt, a panic from x.f = p()
 		// when x is nil is required to come after the side-effects of
@@ -508,7 +509,7 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
 			v.setType(et)
 			return fn.emit(v)
 		}
-		return &lazyAddress{addr: emit, t: deref(et), pos: e.Lbrack, expr: e}
+		return &lazyAddress{addr: emit, t: mustDeref(et), pos: e.Lbrack, expr: e}
 
 	case *ast.StarExpr:
 		return &address{addr: b.expr(fn, e.X), pos: e.Star, expr: e}
@@ -554,7 +555,7 @@ func (b *builder) assign(fn *Function, loc lvalue, e ast.Expr, isZero bool, sb *
 		// so if the type of the location is a pointer,
 		// an &-operation is implied.
 		if _, ok := loc.(blank); !ok { // avoid calling blank.typ()
-			if isPointer(loc.typ()) {
+			if _, ok := deptr(loc.typ()); ok {
 				ptr := b.addr(fn, e, true).address(fn)
 				// copy address
 				if sb != nil {
@@ -584,7 +585,7 @@ func (b *builder) assign(fn *Function, loc lvalue, e ast.Expr, isZero bool, sb *
 
 				// Subtle: emit debug ref for aggregate types only;
 				// slice and map are handled by store ops in compLit.
-				switch loc.typ().Underlying().(type) {
+				switch loc.typ().Underlying().(type) { // TODO(taking): check if Underlying() appropriate.
 				case *types.Struct, *types.Array:
 					emitDebugRef(fn, e, addr, true)
 				}
@@ -831,7 +832,7 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value {
 			// The result is a "bound".
 			obj := sel.obj.(*types.Func)
 			rt := fn.typ(recvType(obj))
-			wantAddr := isPointer(rt)
+			_, wantAddr := deptr(rt)
 			escaping := true
 			v := b.receiver(fn, e.X, wantAddr, escaping, sel)
 
@@ -958,8 +959,9 @@ func (b *builder) stmtList(fn *Function, list []ast.Stmt) {
 //
 // escaping is defined as per builder.addr().
 func (b *builder) receiver(fn *Function, e ast.Expr, wantAddr, escaping bool, sel *selection) Value {
+
 	var v Value
-	if wantAddr && !sel.indirect && !isPointer(fn.typeOf(e)) {
+	if _, eptr := deptr(fn.typeOf(e)); wantAddr && !sel.indirect && !eptr {
 		v = b.addr(fn, e, escaping).address(fn)
 	} else {
 		v = b.expr(fn, e)
@@ -968,7 +970,7 @@ func (b *builder) receiver(fn *Function, e ast.Expr, wantAddr, escaping bool, se
 	last := len(sel.index) - 1
 	// The position of implicit selection is the position of the inducing receiver expression.
 	v = emitImplicitSelections(fn, v, sel.index[:last], e.Pos())
-	if !wantAddr && isPointer(v.Type()) {
+	if _, vptr := deptr(v.Type()); !wantAddr && vptr {
 		v = emitLoad(fn, v)
 	}
 	return v
@@ -987,7 +989,7 @@ func (b *builder) setCallFunc(fn *Function, e *ast.CallExpr, c *CallCommon) {
 			obj := sel.obj.(*types.Func)
 			recv := recvType(obj)
 
-			wantAddr := isPointer(recv)
+			_, wantAddr := deptr(recv)
 			escaping := true
 			v := b.receiver(fn, selector.X, wantAddr, escaping, sel)
 			if types.IsInterface(recv) {
@@ -1253,8 +1255,10 @@ func (b *builder) arrayLen(fn *Function, elts []ast.Expr) int64 {
 // literal has type *T behaves like &T{}.
 // In that case, addr must hold a T, not a *T.
 func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero bool, sb *storebuf) {
-	typ := deref(fn.typeOf(e))                        // type with name [may be type param]
-	t := deref(typeparams.CoreType(typ)).Underlying() // core type for comp lit case
+	typ, _ := deptr(fn.typeOf(e))           // type with name [may be type param]
+	t, _ := deptr(typeparams.CoreType(typ)) // core type for comp lit case
+	t = t.Underlying()
+
 	// Computing typ and t is subtle as these handle pointer types.
 	// For example, &T{...} is valid even for maps and slices.
 	// Also typ should refer to T (not *T) while t should be the core type of T.
@@ -1282,7 +1286,8 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero
 	case *types.Struct:
 		if !isZero && len(e.Elts) != t.NumFields() {
 			// memclear
-			sb.store(&address{addr, e.Lbrace, nil}, zeroConst(deref(addr.Type())))
+			dt, _ := deptr(addr.Type())
+			sb.store(&address{addr, e.Lbrace, nil}, zeroConst(dt))
 			isZero = true
 		}
 		for i, e := range e.Elts {
@@ -1326,7 +1331,8 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero
 
 			if !isZero && int64(len(e.Elts)) != at.Len() {
 				// memclear
-				sb.store(&address{array, e.Lbrace, nil}, zeroConst(deref(array.Type())))
+				dt, _ := deptr(array.Type())
+				sb.store(&address{array, e.Lbrace, nil}, zeroConst(dt))
 			}
 		}
 
@@ -1379,8 +1385,13 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero
 			//	map[*struct{}]bool{{}: true}
 			// An &-operation may be implied:
 			//	map[*struct{}]bool{&struct{}{}: true}
+			wantAddr := false
+			if _, ok := unparen(e.Key).(*ast.CompositeLit); ok {
+				_, wantAddr = t.Key().Underlying().(*types.Pointer)
+			}
+
 			var key Value
-			if _, ok := unparen(e.Key).(*ast.CompositeLit); ok && isPointer(t.Key()) {
+			if wantAddr {
 				// A CompositeLit never evaluates to a pointer,
 				// so if the type of the location is a pointer,
 				// an &-operation is implied.
@@ -1873,7 +1884,8 @@ func (b *builder) rangeIndexed(fn *Function, x Value, tv types.Type, pos token.P
 
 	// Determine number of iterations.
 	var length Value
-	if arr, ok := deref(x.Type()).Underlying().(*types.Array); ok {
+	dt, _ := deptr(x.Type())
+	if arr, ok := dt.Underlying().(*types.Array); ok {
 		// For array or *array, the number of iterations is
 		// known statically thanks to the type.  We avoid a
 		// data dependence upon x, permitting later dead-code
@@ -1882,6 +1894,7 @@ func (b *builder) rangeIndexed(fn *Function, x Value, tv types.Type, pos token.P
 		// We still generate code for x, in case it has effects.
 		//
 		// TypeParams do not have constant length. Use underlying instead of core type.
+		// TODO: check if needed.
 		length = intConst(arr.Len())
 	} else {
 		// length = len(x).

go/ssa/builder_generic_test.go

@@ -5,6 +5,7 @@
 package ssa_test
 
 import (
+	"bytes"
 	"fmt"
 	"go/parser"
 	"go/token"
@@ -540,30 +541,30 @@ func TestInstructionString(t *testing.T) {
 	const contents = `
 	package p
 
-	//@ instrs("f", "*ssa.TypeAssert")
-	//@ instrs("f", "*ssa.Call", "print(nil:interface{}, 0:int)")
-	func f(x int) { // non-generic smoke test.
+	//@ instrs("f0", "*ssa.TypeAssert")
+	//@ instrs("f0", "*ssa.Call", "print(nil:interface{}, 0:int)")
+	func f0(x int) { // non-generic smoke test.
 		var i interface{}
 		print(i, 0)
 	}
 
-	//@ instrs("h", "*ssa.Alloc", "local T (u)")
-	//@ instrs("h", "*ssa.FieldAddr", "&t0.x [#0]")
-	func h[T ~struct{ x string }]() T {
+	//@ instrs("f1", "*ssa.Alloc", "local T (u)")
+	//@ instrs("f1", "*ssa.FieldAddr", "&t0.x [#0]")
+	func f1[T ~struct{ x string }]() T {
 		u := T{"lorem"}
 		return u
 	}
 
-	//@ instrs("c", "*ssa.TypeAssert", "typeassert t0.(interface{})")
-	//@ instrs("c", "*ssa.Call", "invoke x.foo()")
-	func c[T interface{ foo() string }](x T) {
+	//@ instrs("f2", "*ssa.TypeAssert", "typeassert t0.(interface{})")
+	//@ instrs("f2", "*ssa.Call", "invoke x.foo()")
+	func f2[T interface{ foo() string }](x T) {
 		_ = x.foo
 		_ = x.foo()
 	}
 
-	//@ instrs("d", "*ssa.TypeAssert", "typeassert t0.(interface{})")
-	//@ instrs("d", "*ssa.Call", "invoke x.foo()")
-	func d[T interface{ foo() string; comparable }](x T) {
+	//@ instrs("f3", "*ssa.TypeAssert", "typeassert t0.(interface{})")
+	//@ instrs("f3", "*ssa.Call", "invoke x.foo()")
+	func f3[T interface{ foo() string; comparable }](x T) {
 		_ = x.foo
 		_ = x.foo()
 	}
@@ -643,14 +644,16 @@ func TestInstructionString(t *testing.T) {
 
 	// Check each expectation.
 	for key, value := range expectations {
-		if _, ok := p.Members[key.function]; !ok {
+		fn, ok := p.Members[key.function].(*ssa.Function)
+		if !ok {
 			t.Errorf("Expectation on %s does not match a member in %s", key.function, p.Pkg.Name())
 		}
 		got, want := value.matches, value.wants
 		sort.Strings(got)
 		sort.Strings(want)
 		if !reflect.DeepEqual(want, got) {
 			t.Errorf("Within %s wanted instructions of kind %s: %q. got %q", key.function, key.kind, want, got)
+			logFunction(t, fn)
 		}
 	}
 }
@@ -664,3 +667,10 @@ func packageName(t testing.TB, content string) string {
 	}
 	return f.Name.Name
 }
+
+func logFunction(t testing.TB, fn *ssa.Function) {
+	// TODO: Consider adding a ssa.Function.GoString() so this can be logged to t via '%#v'.
+	var buf bytes.Buffer
+	ssa.WriteFunction(&buf, fn)
+	t.Log(buf.String())
+}

go/ssa/emit.go

@@ -29,7 +29,7 @@ func emitNew(f *Function, typ types.Type, pos token.Pos) *Alloc {
 // new temporary, and returns the value so defined.
 func emitLoad(f *Function, addr Value) *UnOp {
 	v := &UnOp{Op: token.MUL, X: addr}
-	v.setType(deref(typeparams.CoreType(addr.Type())))
+	v.setType(mustDeref(addr.Type()))
 	f.emit(v)
 	return v
 }
@@ -478,9 +478,9 @@ func emitTailCall(f *Function, call *Call) {
 // value of a field.
 func emitImplicitSelections(f *Function, v Value, indices []int, pos token.Pos) Value {
 	for _, index := range indices {
-		fld := typeparams.CoreType(deref(v.Type())).(*types.Struct).Field(index)
-
-		if isPointer(v.Type()) {
+		st, vptr := deptr(v.Type())
+		fld := typeparams.CoreType(st).(*types.Struct).Field(index)
+		if vptr {
 			instr := &FieldAddr{
 				X:     v,
 				Field: index,
@@ -489,7 +489,7 @@ func emitImplicitSelections(f *Function, v Value, indices []int, pos token.Pos)
 			instr.setType(types.NewPointer(fld.Type()))
 			v = f.emit(instr)
 			// Load the field's value iff indirectly embedded.
-			if isPointer(fld.Type()) {
+			if _, fldptr := deptr(fld.Type()); fldptr {
 				v = emitLoad(f, v)
 			}
 		} else {
@@ -512,8 +512,15 @@ func emitImplicitSelections(f *Function, v Value, indices []int, pos token.Pos)
 // field's value.
 // Ident id is used for position and debug info.
 func emitFieldSelection(f *Function, v Value, index int, wantAddr bool, id *ast.Ident) Value {
-	fld := typeparams.CoreType(deref(v.Type())).(*types.Struct).Field(index)
-	if isPointer(v.Type()) {
+	// TODO(taking): Cover the following cases of interest
+	// 	func f[T any, S struct{f T}, P *struct{f T}, PS *S](x T) {
+	// 		_ := S{f: x}
+	//      _ := P{f: x}
+	//      _ := PS{f: x}
+	//  }
+	st, vptr := deptr(v.Type())
+	fld := typeparams.CoreType(st).(*types.Struct).Field(index)
+	if vptr {
 		instr := &FieldAddr{
 			X:     v,
 			Field: index,

go/ssa/func.go

@@ -596,7 +596,7 @@ func WriteFunction(buf *bytes.Buffer, f *Function) {
 	if len(f.Locals) > 0 {
 		buf.WriteString("# Locals:\n")
 		for i, l := range f.Locals {
-			fmt.Fprintf(buf, "# % 3d:\t%s %s\n", i, l.Name(), relType(deref(l.Type()), from))
+			fmt.Fprintf(buf, "# % 3d:\t%s %s\n", i, l.Name(), relType(mustDeref(l.Type()), from))
 		}
 	}
 	writeSignature(buf, from, f.Name(), f.Signature, f.Params)

go/ssa/lift.go

@@ -460,7 +460,7 @@ func liftAlloc(df domFrontier, alloc *Alloc, newPhis newPhiMap, fresh *int) bool
 				*fresh++
 
 				phi.pos = alloc.Pos()
-				phi.setType(deref(alloc.Type()))
+				phi.setType(mustDeref(alloc.Type()))
 				phi.block = v
 				if debugLifting {
 					fmt.Fprintf(os.Stderr, "\tplace %s = %s at block %s\n", phi.Name(), phi, v)
@@ -505,7 +505,7 @@ func replaceAll(x, y Value) {
 func renamed(renaming []Value, alloc *Alloc) Value {
 	v := renaming[alloc.index]
 	if v == nil {
-		v = zeroConst(deref(alloc.Type()))
+		v = zeroConst(mustDeref(alloc.Type()))
 		renaming[alloc.index] = v
 	}
 	return v

go/ssa/methods.go

@@ -101,8 +101,11 @@ func (prog *Program) addMethod(mset *methodSet, sel *types.Selection, cr *creato
 		sel := toSelection(sel)
 		obj := sel.obj.(*types.Func)
 
+		_, ptrObj := deptr(recvType(obj))
+		_, ptrRecv := deptr(sel.recv)
+
 		needsPromotion := len(sel.index) > 1
-		needsIndirection := !isPointer(recvType(obj)) && isPointer(sel.recv)
+		needsIndirection := !ptrObj && ptrRecv
 		if needsPromotion || needsIndirection {
 			fn = makeWrapper(prog, sel, cr)
 		} else {

go/ssa/print.go

@@ -95,7 +95,7 @@ func (v *Alloc) String() string {
 		op = "new"
 	}
 	from := v.Parent().relPkg()
-	return fmt.Sprintf("%s %s (%s)", op, relType(deref(v.Type()), from), v.Comment)
+	return fmt.Sprintf("%s %s (%s)", op, relType(mustDeref(v.Type()), from), v.Comment)
 }
 
 func (v *Phi) String() string {
@@ -259,7 +259,8 @@ func (v *MakeChan) String() string {
 }
 
 func (v *FieldAddr) String() string {
-	st := typeparams.CoreType(deref(v.X.Type())).(*types.Struct)
+	dt, _ := deptr(v.X.Type())
+	st := typeparams.CoreType(dt).(*types.Struct)
 	// Be robust against a bad index.
 	name := "?"
 	if 0 <= v.Field && v.Field < st.NumFields() {
@@ -452,7 +453,7 @@ func WritePackage(buf *bytes.Buffer, p *Package) {
 
 		case *Global:
 			fmt.Fprintf(buf, "  var   %-*s %s\n",
-				maxname, name, relType(mem.Type().(*types.Pointer).Elem(), from))
+				maxname, name, relType(mustDeref(mem.Type()), from))
 		}
 	}
 

go/ssa/subst.go

@@ -249,7 +249,7 @@ func (subst *subster) interface_(iface *types.Interface) *types.Interface {
 	}
 
 	// methods for the interface. Initially nil if there is no known change needed.
-	// Signatures for the method where recv is nil. NewInterfaceType fills in the recievers.
+	// Signatures for the method where recv is nil. NewInterfaceType fills in the receivers.
 	var methods []*types.Func
 	initMethods := func(n int) { // copy first n explicit methods
 		methods = make([]*types.Func, iface.NumExplicitMethods())
@@ -262,7 +262,7 @@ func (subst *subster) interface_(iface *types.Interface) *types.Interface {
 	for i := 0; i < iface.NumExplicitMethods(); i++ {
 		f := iface.ExplicitMethod(i)
 		// On interfaces, we need to cycle break on anonymous interface types
-		// being in a cycle with their signatures being in cycles with their recievers
+		// being in a cycle with their signatures being in cycles with their receivers
 		// that do not go through a Named.
 		norecv := changeRecv(f.Type().(*types.Signature), nil)
 		sig := subst.typ(norecv)