internal/typesparams: add Deref

This change defines Deref(T), which returns the type of
the variable pointed to by T if its core type is a pointer,
or T otherwise, and removes all the various other flavors
of 'deref' helper that exist across the repo.

Also fix and test a generics bug in fillstruct.

Updates golang/go#65294

Change-Id: I14f6f35b58eefbad316391745745d662b061c013
Reviewed-on: https://go-review.googlesource.com/c/tools/+/565456
Reviewed-by: Tim King <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

go/analysis/passes/composite/composite.go

@@ -84,10 +84,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		default:
 			structuralTypes = append(structuralTypes, typ)
 		}
+
 		for _, typ := range structuralTypes {
-			// TODO(adonovan): this operation is questionable.
-			under := aliases.Unalias(deref(typ.Underlying()))
-			strct, ok := under.(*types.Struct)
+			strct, ok := typeparams.Deref(typ).Underlying().(*types.Struct)
 			if !ok {
 				// skip non-struct composite literals
 				continue
@@ -144,19 +143,6 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
-// Note: this is not the usual deref operator!
-// It strips off all Pointer constructors (and their Aliases).
-func deref(typ types.Type) types.Type {
-	for {
-		ptr, ok := aliases.Unalias(typ).(*types.Pointer)
-		if !ok {
-			break
-		}
-		typ = ptr.Elem().Underlying()
-	}
-	return typ
-}
-
 // isLocalType reports whether typ belongs to the same package as pass.
 // TODO(adonovan): local means "internal to a function"; rename to isSamePackageType.
 func isLocalType(pass *analysis.Pass, typ types.Type) bool {

go/analysis/passes/unusedwrite/unusedwrite.go

@@ -6,14 +6,14 @@ package unusedwrite
 
 import (
 	_ "embed"
-	"fmt"
 	"go/types"
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
 	"golang.org/x/tools/go/ssa"
 	"golang.org/x/tools/internal/aliases"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 //go:embed doc.go
@@ -37,9 +37,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		for _, store := range reports {
 			switch addr := store.Addr.(type) {
 			case *ssa.FieldAddr:
+				field := typeparams.CoreType(typeparams.MustDeref(addr.X.Type())).(*types.Struct).Field(addr.Field)
 				pass.Reportf(store.Pos(),
-					"unused write to field %s",
-					getFieldName(addr.X.Type(), addr.Field))
+					"unused write to field %s", field.Name())
 			case *ssa.IndexAddr:
 				pass.Reportf(store.Pos(),
 					"unused write to array index %s", addr.Index)
@@ -151,21 +151,3 @@ func hasStructOrArrayType(v ssa.Value) bool {
 	}
 	return isStructOrArray(v.Type())
 }
-
-// getFieldName returns the name of a field in a struct.
-// It the field is not found, then it returns the string format of the index.
-//
-// For example, for struct T {x int, y int), getFieldName(*T, 1) returns "y".
-func getFieldName(tp types.Type, index int) string {
-	// TODO(adonovan): use
-	//   stp, ok := typeparams.Deref(tp).Underlying().(*types.Struct); ok {
-	// when Deref is defined. But see CL 565456 for a better fix.
-
-	if pt, ok := aliases.Unalias(tp).(*types.Pointer); ok {
-		tp = pt.Elem()
-	}
-	if stp, ok := tp.Underlying().(*types.Struct); ok {
-		return stp.Field(index).Name()
-	}
-	return fmt.Sprintf("%d", index)
-}

go/internal/gccgoimporter/parser.go

@@ -22,6 +22,7 @@ import (
 	"unicode/utf8"
 
 	"golang.org/x/tools/internal/aliases"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 type parser struct {
@@ -242,13 +243,6 @@ func (p *parser) parseName() string {
 	return name
 }
 
-func deref(typ types.Type) types.Type {
-	if p, _ := aliases.Unalias(typ).(*types.Pointer); p != nil {
-		typ = p.Elem()
-	}
-	return typ
-}
-
 // parseField parses a Field:
 //
 //	Field = Name Type [string] .
@@ -262,7 +256,7 @@ func (p *parser) parseField(pkg *types.Package) (field *types.Var, tag string) {
 		if aname, ok := p.aliases[n]; ok {
 			name = aname
 		} else {
-			switch typ := aliases.Unalias(deref(typ)).(type) {
+			switch typ := aliases.Unalias(typesinternal.Unpointer(typ)).(type) {
 			case *types.Basic:
 				name = typ.Name()
 			case *types.Named:

go/ssa/builder.go

@@ -336,7 +336,7 @@ func (b *builder) builtin(fn *Function, obj *types.Builtin, args []ast.Expr, typ
 		// We must still evaluate the value, though.  (If it
 		// was side-effect free, the whole call would have
 		// been constant-folded.)
-		t, _ := deref(fn.typeOf(args[0]))
+		t := typeparams.Deref(fn.typeOf(args[0]))
 		if at, ok := typeparams.CoreType(t).(*types.Array); ok {
 			b.expr(fn, args[0]) // for effects only
 			return intConst(at.Len())
@@ -392,7 +392,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:
-		typ, _ := deref(fn.typeOf(e))
+		typ := typeparams.Deref(fn.typeOf(e))
 		var v *Alloc
 		if escaping {
 			v = emitNew(fn, typ, e.Lbrace, "complit")
@@ -513,17 +513,15 @@ func (b *builder) assign(fn *Function, loc lvalue, e ast.Expr, isZero bool, sb *
 		// A CompositeLit never evaluates to a pointer,
 		// so if the type of the location is a pointer,
 		// an &-operation is implied.
-		if _, ok := loc.(blank); !ok { // avoid calling blank.typ()
-			if _, ok := deref(loc.typ()); ok {
-				ptr := b.addr(fn, e, true).address(fn)
-				// copy address
-				if sb != nil {
-					sb.store(loc, ptr)
-				} else {
-					loc.store(fn, ptr)
-				}
-				return
+		if !is[blank](loc) && isPointerCore(loc.typ()) { // avoid calling blank.typ()
+			ptr := b.addr(fn, e, true).address(fn)
+			// copy address
+			if sb != nil {
+				sb.store(loc, ptr)
+			} else {
+				loc.store(fn, ptr)
 			}
+			return
 		}
 
 		if _, ok := loc.(*address); ok {
@@ -795,7 +793,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 := deref(rt)
+			wantAddr := isPointer(rt)
 			escaping := true
 			v := b.receiver(fn, e.X, wantAddr, escaping, sel)
 
@@ -923,7 +921,7 @@ 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 _, eptr := deref(fn.typeOf(e)); wantAddr && !sel.indirect && !eptr {
+	if wantAddr && !sel.indirect && !isPointerCore(fn.typeOf(e)) {
 		v = b.addr(fn, e, escaping).address(fn)
 	} else {
 		v = b.expr(fn, e)
@@ -935,7 +933,7 @@ func (b *builder) receiver(fn *Function, e ast.Expr, wantAddr, escaping bool, se
 	if types.IsInterface(v.Type()) {
 		// When v is an interface, sel.Kind()==MethodValue and v.f is invoked.
 		// So v is not loaded, even if v has a pointer core type.
-	} else if _, vptr := deref(v.Type()); !wantAddr && vptr {
+	} else if !wantAddr && isPointerCore(v.Type()) {
 		v = emitLoad(fn, v)
 	}
 	return v
@@ -954,7 +952,7 @@ func (b *builder) setCallFunc(fn *Function, e *ast.CallExpr, c *CallCommon) {
 			obj := sel.obj.(*types.Func)
 			recv := recvType(obj)
 
-			_, wantAddr := deref(recv)
+			wantAddr := isPointer(recv)
 			escaping := true
 			v := b.receiver(fn, selector.X, wantAddr, escaping, sel)
 			if types.IsInterface(recv) {
@@ -1215,12 +1213,12 @@ 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]
+	typ := typeparams.Deref(fn.typeOf(e)) // retain the named/alias/param type, if any
 	switch t := typeparams.CoreType(typ).(type) {
 	case *types.Struct:
 		if !isZero && len(e.Elts) != t.NumFields() {
 			// memclear
-			zt, _ := deref(addr.Type())
+			zt := typeparams.MustDeref(addr.Type())
 			sb.store(&address{addr, e.Lbrace, nil}, zeroConst(zt))
 			isZero = true
 		}
@@ -1263,7 +1261,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero
 
 			if !isZero && int64(len(e.Elts)) != at.Len() {
 				// memclear
-				zt, _ := deref(array.Type())
+				zt := typeparams.MustDeref(array.Type())
 				sb.store(&address{array, e.Lbrace, nil}, zeroConst(zt))
 			}
 		}
@@ -1319,7 +1317,7 @@ func (b *builder) compLit(fn *Function, addr Value, e *ast.CompositeLit, isZero
 			//	map[*struct{}]bool{&struct{}{}: true}
 			wantAddr := false
 			if _, ok := unparen(e.Key).(*ast.CompositeLit); ok {
-				_, wantAddr = deref(t.Key())
+				wantAddr = isPointerCore(t.Key())
 			}
 
 			var key Value
@@ -1992,7 +1990,7 @@ func (b *builder) rangeIndexed(fn *Function, x Value, tv types.Type, pos token.P
 
 	// Determine number of iterations.
 	var length Value
-	dt, _ := deref(x.Type())
+	dt := typeparams.Deref(x.Type())
 	if arr, ok := typeparams.CoreType(dt).(*types.Array); ok {
 		// For array or *array, the number of iterations is
 		// known statically thanks to the type.  We avoid a

go/ssa/emit.go

@@ -524,8 +524,8 @@ 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 {
-		if st, vptr := deref(v.Type()); vptr {
-			fld := fieldOf(st, index)
+		if isPointerCore(v.Type()) {
+			fld := fieldOf(typeparams.MustDeref(v.Type()), index)
 			instr := &FieldAddr{
 				X:     v,
 				Field: index,
@@ -534,7 +534,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 _, fldptr := deref(fld.Type()); fldptr {
+			if isPointerCore(fld.Type()) {
 				v = emitLoad(f, v)
 			}
 		} else {
@@ -558,8 +558,8 @@ 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 {
-	if st, vptr := deref(v.Type()); vptr {
-		fld := fieldOf(st, index)
+	if isPointerCore(v.Type()) {
+		fld := fieldOf(typeparams.MustDeref(v.Type()), index)
 		instr := &FieldAddr{
 			X:     v,
 			Field: index,

go/ssa/interp/interp.go

@@ -54,6 +54,7 @@ import (
 	"sync/atomic"
 
 	"golang.org/x/tools/go/ssa"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 type continuation int
@@ -245,7 +246,7 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
 		fr.get(instr.Chan).(chan value) <- fr.get(instr.X)
 
 	case *ssa.Store:
-		store(deref(instr.Addr.Type()), fr.get(instr.Addr).(*value), fr.get(instr.Val))
+		store(typeparams.MustDeref(instr.Addr.Type()), fr.get(instr.Addr).(*value), fr.get(instr.Val))
 
 	case *ssa.If:
 		succ := 1
@@ -289,7 +290,7 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
 			// local
 			addr = fr.env[instr].(*value)
 		}
-		*addr = zero(deref(instr.Type()))
+		*addr = zero(typeparams.MustDeref(instr.Type()))
 
 	case *ssa.MakeSlice:
 		slice := make([]value, asInt64(fr.get(instr.Cap)))
@@ -528,7 +529,7 @@ func callSSA(i *interpreter, caller *frame, callpos token.Pos, fn *ssa.Function,
 	fr.block = fn.Blocks[0]
 	fr.locals = make([]value, len(fn.Locals))
 	for i, l := range fn.Locals {
-		fr.locals[i] = zero(deref(l.Type()))
+		fr.locals[i] = zero(typeparams.MustDeref(l.Type()))
 		fr.env[l] = &fr.locals[i]
 	}
 	for i, p := range fn.Params {
@@ -673,7 +674,7 @@ func Interpret(mainpkg *ssa.Package, mode Mode, sizes types.Sizes, filename stri
 		for _, m := range pkg.Members {
 			switch v := m.(type) {
 			case *ssa.Global:
-				cell := zero(deref(v.Type()))
+				cell := zero(typeparams.MustDeref(v.Type()))
 				i.globals[v] = &cell
 			}
 		}
@@ -717,12 +718,3 @@ func Interpret(mainpkg *ssa.Package, mode Mode, sizes types.Sizes, filename stri
 	}
 	return
 }
-
-// deref returns a pointer's element type; otherwise it returns typ.
-// TODO(adonovan): Import from ssa?
-func deref(typ types.Type) types.Type {
-	if p, ok := typ.Underlying().(*types.Pointer); ok {
-		return p.Elem()
-	}
-	return typ
-}

go/ssa/interp/ops.go

@@ -18,6 +18,7 @@ import (
 
 	"golang.org/x/tools/go/ssa"
 	"golang.org/x/tools/internal/aliases"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 // If the target program panics, the interpreter panics with this type.
@@ -884,7 +885,7 @@ func unop(instr *ssa.UnOp, x value) value {
 			return -x
 		}
 	case token.MUL:
-		return load(deref(instr.X.Type()), x.(*value))
+		return load(typeparams.MustDeref(instr.X.Type()), x.(*value))
 	case token.NOT:
 		return !x.(bool)
 	case token.XOR:

go/ssa/methods.go

@@ -58,10 +58,8 @@ func (prog *Program) MethodValue(sel *types.Selection) *Function {
 		fn, ok := mset.mapping[id]
 		if !ok {
 			obj := sel.Obj().(*types.Func)
-			_, ptrObj := deptr(recvType(obj))
-			_, ptrRecv := deptr(T)
 			needsPromotion := len(sel.Index()) > 1
-			needsIndirection := !ptrObj && ptrRecv
+			needsIndirection := !isPointer(recvType(obj)) && isPointer(T)
 			if needsPromotion || needsIndirection {
 				fn = createWrapper(prog, toSelection(sel), &cr)
 			} else {

go/ssa/util.go

@@ -101,24 +101,22 @@ func isBasicConvTypes(tset termList) bool {
 	return all && basics >= 1 && tset.Len()-basics <= 1
 }
 
-// deptr returns a pointer's element type and true; otherwise it returns (typ, false).
-// This function is oblivious to core types and is not suitable for generics.
+// isPointer reports whether t's underlying type is a pointer.
+func isPointer(t types.Type) bool {
+	return is[*types.Pointer](t.Underlying())
+}
+
+// isPointerCore reports whether t's core type is a pointer.
 //
-// TODO: Deprecate this function once all usages have been audited.
-func deptr(typ types.Type) (types.Type, bool) {
-	if p, ok := typ.Underlying().(*types.Pointer); ok {
-		return p.Elem(), true
-	}
-	return typ, false
+// (Most pointer manipulation is related to receivers, in which case
+// isPointer is appropriate. tecallers can use isPointer(t).
+func isPointerCore(t types.Type) bool {
+	return is[*types.Pointer](typeparams.CoreType(t))
 }
 
-// deref returns the element type of a type with a pointer core type and true;
-// otherwise it returns (typ, false).
-func deref(typ types.Type) (types.Type, bool) {
-	if p, ok := typeparams.CoreType(typ).(*types.Pointer); ok {
-		return p.Elem(), true
-	}
-	return typ, false
+func is[T any](x any) bool {
+	_, ok := x.(T)
+	return ok
 }
 
 // recvType returns the receiver type of method obj.