From 71acab9a7f9f83fb6436c640caf7f3e744d3bef5 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 20 Feb 2024 14:55:45 -0500 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley --- go/analysis/passes/composite/composite.go | 18 +-------- go/analysis/passes/unusedwrite/unusedwrite.go | 24 ++--------- go/internal/gccgoimporter/parser.go | 10 +---- go/ssa/builder.go | 40 +++++++++---------- go/ssa/emit.go | 10 ++--- go/ssa/interp/interp.go | 18 +++------ go/ssa/interp/ops.go | 3 +- go/ssa/methods.go | 4 +- go/ssa/util.go | 30 +++++++------- go/ssa/wrappers.go | 12 +++--- .../analysis/fillstruct/fillstruct.go | 17 ++------ .../testdata/src/typeparams/typeparams.go | 4 ++ gopls/internal/golang/completion/util.go | 1 + internal/refactor/inline/callee.go | 10 +---- internal/refactor/inline/falcon.go | 8 ++-- internal/refactor/inline/inline.go | 5 +-- internal/refactor/inline/util.go | 10 +++-- internal/typeparams/coretype.go | 15 +++++++ refactor/rename/check.go | 24 +++++------ refactor/rename/util.go | 5 +++ refactor/satisfy/find.go | 12 +----- 21 files changed, 113 insertions(+), 167 deletions(-) diff --git a/go/analysis/passes/composite/composite.go b/go/analysis/passes/composite/composite.go index 6b126f897..8cc6c4a05 100644 --- a/go/analysis/passes/composite/composite.go +++ b/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 { diff --git a/go/analysis/passes/unusedwrite/unusedwrite.go b/go/analysis/passes/unusedwrite/unusedwrite.go index a01cbb8f8..a99c54833 100644 --- a/go/analysis/passes/unusedwrite/unusedwrite.go +++ b/go/analysis/passes/unusedwrite/unusedwrite.go @@ -6,7 +6,6 @@ package unusedwrite import ( _ "embed" - "fmt" "go/types" "golang.org/x/tools/go/analysis" @@ -14,6 +13,7 @@ import ( "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) -} diff --git a/go/internal/gccgoimporter/parser.go b/go/internal/gccgoimporter/parser.go index dc40d217a..b0eb1ddf8 100644 --- a/go/internal/gccgoimporter/parser.go +++ b/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: diff --git a/go/ssa/builder.go b/go/ssa/builder.go index 9b3753c39..1f7f364ee 100644 --- a/go/ssa/builder.go +++ b/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 diff --git a/go/ssa/emit.go b/go/ssa/emit.go index 97c0b5995..716299ffe 100644 --- a/go/ssa/emit.go +++ b/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, diff --git a/go/ssa/interp/interp.go b/go/ssa/interp/interp.go index 79363f573..f677ba2b6 100644 --- a/go/ssa/interp/interp.go +++ b/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 -} diff --git a/go/ssa/interp/ops.go b/go/ssa/interp/ops.go index 668fcae61..99eab86e1 100644 --- a/go/ssa/interp/ops.go +++ b/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: diff --git a/go/ssa/methods.go b/go/ssa/methods.go index 197b9e7c2..58bd45b81 100644 --- a/go/ssa/methods.go +++ b/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 { diff --git a/go/ssa/util.go b/go/ssa/util.go index 462e27507..314ca2b6f 100644 --- a/go/ssa/util.go +++ b/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. -// -// 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 +// isPointer reports whether t's underlying type is a pointer. +func isPointer(t types.Type) bool { + return is[*types.Pointer](t.Underlying()) } -// 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 +// isPointerCore reports whether t's core type is a pointer. +// +// (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)) +} + +func is[T any](x any) bool { + _, ok := x.(T) + return ok } // recvType returns the receiver type of method obj. diff --git a/go/ssa/wrappers.go b/go/ssa/wrappers.go index 7c7ee4099..b25c4c789 100644 --- a/go/ssa/wrappers.go +++ b/go/ssa/wrappers.go @@ -24,6 +24,8 @@ import ( "go/token" "go/types" + + "golang.org/x/tools/internal/typeparams" ) // -- wrappers ----------------------------------------------------------- @@ -97,14 +99,12 @@ func (b *builder) buildWrapper(fn *Function) { indices := fn.method.index var v Value = fn.Locals[0] // spilled receiver - srdt, ptrRecv := deptr(fn.method.recv) - if ptrRecv { + if isPointer(fn.method.recv) { v = emitLoad(fn, v) // For simple indirection wrappers, perform an informative nil-check: // "value method (T).f called using nil *T pointer" - _, ptrObj := deptr(recvType(fn.object)) - if len(indices) == 1 && !ptrObj { + if len(indices) == 1 && !isPointer(recvType(fn.object)) { var c Call c.Call.Value = &Builtin{ name: "ssa:wrapnilchk", @@ -114,7 +114,7 @@ func (b *builder) buildWrapper(fn *Function) { } c.Call.Args = []Value{ v, - stringConst(srdt.String()), + stringConst(typeparams.MustDeref(fn.method.recv).String()), stringConst(fn.method.obj.Name()), } c.setType(v.Type()) @@ -138,7 +138,7 @@ func (b *builder) buildWrapper(fn *Function) { var c Call if r := recvType(fn.object); !types.IsInterface(r) { // concrete method - if _, ptrObj := deptr(r); !ptrObj { + if !isPointer(r) { v = emitLoad(fn, v) } c.Call.Value = fn.Prog.objectMethod(fn.object, b.created) diff --git a/gopls/internal/analysis/fillstruct/fillstruct.go b/gopls/internal/analysis/fillstruct/fillstruct.go index 1f8183fdc..785b508d5 100644 --- a/gopls/internal/analysis/fillstruct/fillstruct.go +++ b/gopls/internal/analysis/fillstruct/fillstruct.go @@ -28,6 +28,7 @@ import ( "golang.org/x/tools/gopls/internal/util/safetoken" "golang.org/x/tools/internal/analysisinternal" "golang.org/x/tools/internal/fuzzy" + "golang.org/x/tools/internal/typeparams" ) // Diagnose computes diagnostics for fillable struct literals overlapping with @@ -53,8 +54,8 @@ func Diagnose(inspect *inspector.Inspector, start, end token.Pos, pkg *types.Pac } // Find reference to the type declaration of the struct being initialized. - typ = deref(typ) - tStruct, ok := typ.Underlying().(*types.Struct) + typ = typeparams.Deref(typ) + tStruct, ok := typeparams.CoreType(typ).(*types.Struct) if !ok { return } @@ -150,7 +151,7 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, fil } // Find reference to the type declaration of the struct being initialized. - typ = deref(typ) + typ = typeparams.Deref(typ) tStruct, ok := typ.Underlying().(*types.Struct) if !ok { return nil, nil, fmt.Errorf("%s is not a (pointer to) struct type", @@ -490,13 +491,3 @@ func populateValue(f *ast.File, pkg *types.Package, typ types.Type) ast.Expr { } return nil } - -func deref(t types.Type) types.Type { - for { - ptr, ok := t.Underlying().(*types.Pointer) - if !ok { - return t - } - t = ptr.Elem() - } -} diff --git a/gopls/internal/analysis/fillstruct/testdata/src/typeparams/typeparams.go b/gopls/internal/analysis/fillstruct/testdata/src/typeparams/typeparams.go index d9e3da44a..24e8a930d 100644 --- a/gopls/internal/analysis/fillstruct/testdata/src/typeparams/typeparams.go +++ b/gopls/internal/analysis/fillstruct/testdata/src/typeparams/typeparams.go @@ -48,3 +48,7 @@ func Test() { _ = test } } + +func _[T twoArgStruct[int, int]]() { + _ = T{} // want "T literal has missing fields" +} diff --git a/gopls/internal/golang/completion/util.go b/gopls/internal/golang/completion/util.go index 8ac7f161e..a16537746 100644 --- a/gopls/internal/golang/completion/util.go +++ b/gopls/internal/golang/completion/util.go @@ -132,6 +132,7 @@ func resolveInvalid(fset *token.FileSet, obj types.Object, node ast.Node, info * } func isPointer(T types.Type) bool { + // TODO(adonovan): use CoreType(T). _, ok := T.(*types.Pointer) return ok } diff --git a/internal/refactor/inline/callee.go b/internal/refactor/inline/callee.go index c9a7ea0c8..e56205923 100644 --- a/internal/refactor/inline/callee.go +++ b/internal/refactor/inline/callee.go @@ -149,7 +149,7 @@ func AnalyzeCallee(logf func(string, ...any), fset *token.FileSet, pkg *types.Pa case *ast.CompositeLit: // Check for struct literals that refer to unexported fields, // whether keyed or unkeyed. (Logic assumes well-typedness.) - litType := deref(info.TypeOf(n)) + litType := typeparams.Deref(info.TypeOf(n)) if s, ok := typeparams.CoreType(litType).(*types.Struct); ok { if n.Type != nil { visit(n.Type) @@ -496,14 +496,6 @@ func addShadows(shadows map[string]bool, info *types.Info, exclude string, stack return shadows } -// deref removes a pointer type constructor from the core type of t. -func deref(t types.Type) types.Type { - if ptr, ok := typeparams.CoreType(t).(*types.Pointer); ok { - return ptr.Elem() - } - return t -} - func isField(obj types.Object) bool { if v, ok := obj.(*types.Var); ok && v.IsField() { return true diff --git a/internal/refactor/inline/falcon.go b/internal/refactor/inline/falcon.go index b49668eb4..de054342b 100644 --- a/internal/refactor/inline/falcon.go +++ b/internal/refactor/inline/falcon.go @@ -423,7 +423,7 @@ func (st *falconState) expr(e ast.Expr) (res any) { // = types.TypeAndValue | as if e.Type != nil { _ = st.expr(e.Type) } - t := deref(typeparams.CoreType(deref(tv.Type))) + t := aliases.Unalias(typeparams.Deref(tv.Type)) var uniques []ast.Expr for _, elt := range e.Elts { if kv, ok := elt.(*ast.KeyValueExpr); ok { @@ -447,7 +447,7 @@ func (st *falconState) expr(e ast.Expr) (res any) { // = types.TypeAndValue | as // - for an array or *array, use [n]int. // The last two entail progressively stronger index checks. var ct ast.Expr // type syntax for constraint - switch t := aliases.Unalias(t).(type) { + switch t := t.(type) { case *types.Map: if types.IsInterface(t.Key()) { ct = &ast.MapType{ @@ -508,7 +508,7 @@ func (st *falconState) expr(e ast.Expr) (res any) { // = types.TypeAndValue | as if kX != nil { // string x = st.toExpr(kX) - } else if arr, ok := deref(st.info.TypeOf(e.X).Underlying()).(*types.Array); ok { + } else if arr, ok := typeparams.CoreType(typeparams.Deref(st.info.TypeOf(e.X))).(*types.Array); ok { // array, *array x = &ast.CompositeLit{ Type: &ast.ArrayType{ @@ -573,7 +573,7 @@ func (st *falconState) expr(e ast.Expr) (res any) { // = types.TypeAndValue | as if kX != nil { // string x = st.toExpr(kX) - } else if arr, ok := deref(st.info.TypeOf(e.X).Underlying()).(*types.Array); ok { + } else if arr, ok := typeparams.CoreType(typeparams.Deref(st.info.TypeOf(e.X))).(*types.Array); ok { // array, *array x = &ast.CompositeLit{ Type: &ast.ArrayType{ diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go index c4f6ac14e..c7ffbb215 100644 --- a/internal/refactor/inline/inline.go +++ b/internal/refactor/inline/inline.go @@ -1134,8 +1134,7 @@ func arguments(caller *Caller, calleeDecl *ast.FuncDecl, assign1 func(*types.Var // updating arg.{expr,typ}. indices := seln.Index() for _, index := range indices[:len(indices)-1] { - t := deref(arg.typ) - fld := typeparams.CoreType(t).(*types.Struct).Field(index) + fld := typeparams.CoreType(typeparams.Deref(arg.typ)).(*types.Struct).Field(index) if fld.Pkg() != caller.Types && !fld.Exported() { return nil, fmt.Errorf("in %s, implicit reference to unexported field .%s cannot be made explicit", debugFormatNode(caller.Fset, caller.Call.Fun), @@ -1162,7 +1161,7 @@ func arguments(caller *Caller, calleeDecl *ast.FuncDecl, assign1 func(*types.Var } else if argIsPtr && !paramIsPtr { // *recv arg.expr = &ast.StarExpr{X: arg.expr} - arg.typ = deref(arg.typ) + arg.typ = typeparams.Deref(arg.typ) arg.duplicable = false arg.pure = false } diff --git a/internal/refactor/inline/util.go b/internal/refactor/inline/util.go index 7581ca29a..475cc7141 100644 --- a/internal/refactor/inline/util.go +++ b/internal/refactor/inline/util.go @@ -120,8 +120,10 @@ func convert(T, x ast.Expr) *ast.CallExpr { } } -// isPointer reports whether t is a pointer type. -func isPointer(t types.Type) bool { return t != deref(t) } +// isPointer reports whether t's core type is a pointer. +func isPointer(t types.Type) bool { + return is[*types.Pointer](typeparams.CoreType(t)) +} // indirectSelection is like seln.Indirect() without bug #8353. func indirectSelection(seln *types.Selection) bool { @@ -150,9 +152,9 @@ func effectiveReceiver(seln *types.Selection) (types.Type, bool) { indices := seln.Index() indirect := false for _, index := range indices[:len(indices)-1] { - if tElem := deref(t); tElem != t { + if isPointer(t) { indirect = true - t = tElem + t = typeparams.MustDeref(t) } t = typeparams.CoreType(t).(*types.Struct).Field(index).Type() } diff --git a/internal/typeparams/coretype.go b/internal/typeparams/coretype.go index e66e9d0f4..24933e43d 100644 --- a/internal/typeparams/coretype.go +++ b/internal/typeparams/coretype.go @@ -124,6 +124,21 @@ func _NormalTerms(typ types.Type) ([]*types.Term, error) { } } +// Deref returns the type of the variable pointed to by t, +// if t's core type is a pointer; otherwise it returns t. +// +// Do not assume that Deref(T)==T implies T is not a pointer: +// consider "type T *T", for example. +// +// TODO(adonovan): ideally this would live in typesinternal, but that +// creates an import cycle. Move there when we melt this package down. +func Deref(t types.Type) types.Type { + if ptr, ok := CoreType(t).(*types.Pointer); ok { + return ptr.Elem() + } + return t +} + // MustDeref returns the type of the variable pointed to by t. // It panics if t's core type is not a pointer. // diff --git a/refactor/rename/check.go b/refactor/rename/check.go index ac2c5d420..f2bd8809e 100644 --- a/refactor/rename/check.go +++ b/refactor/rename/check.go @@ -14,6 +14,8 @@ import ( "golang.org/x/tools/go/loader" "golang.org/x/tools/internal/aliases" + "golang.org/x/tools/internal/typeparams" + "golang.org/x/tools/internal/typesinternal" "golang.org/x/tools/refactor/satisfy" ) @@ -344,7 +346,7 @@ func forEachLexicalRef(info *loader.PackageInfo, obj types.Object, fn func(id *a // Handle recursion ourselves for struct literals // so we don't visit field identifiers. tv := info.Types[n] - if _, ok := deref(tv.Type).Underlying().(*types.Struct); ok { + if is[*types.Struct](typeparams.CoreType(typeparams.Deref(tv.Type))) { if n.Type != nil { ast.Inspect(n.Type, visit) } @@ -463,16 +465,17 @@ func (r *renamer) checkStructField(from *types.Var) { } } - // Renaming an anonymous field requires renaming the type too. e.g. + // Renaming an anonymous field requires renaming the TypeName too. e.g. // print(s.T) // if we rename T to U, // type T int // this and // var s struct {T} // this must change too. if from.Anonymous() { - // TODO(adonovan): think carefully about aliases. - if named, ok := aliases.Unalias(from.Type()).(*types.Named); ok { - r.check(named.Obj()) - } else if named, ok := aliases.Unalias(deref(from.Type())).(*types.Named); ok { - r.check(named.Obj()) + // hasTypeName = {Named,Alias,TypeParam}, though + // a TypeParam cannot appear as an anonymous field. + // TODO(adonovan): add test of aliases. + type hasTypeName interface{ Obj() *types.TypeName } + if t, ok := typesinternal.Unpointer(from.Type()).(hasTypeName); ok { + r.check(t.Obj()) } } @@ -851,10 +854,3 @@ func someUse(info *loader.PackageInfo, obj types.Object) *ast.Ident { // -- Plundered from golang.org/x/tools/go/ssa ----------------- func isInterface(T types.Type) bool { return types.IsInterface(T) } - -func deref(typ types.Type) types.Type { - if p, _ := aliases.Unalias(typ).(*types.Pointer); p != nil { - return p.Elem() - } - return typ -} diff --git a/refactor/rename/util.go b/refactor/rename/util.go index 258ba786c..26926a86f 100644 --- a/refactor/rename/util.go +++ b/refactor/rename/util.go @@ -102,3 +102,8 @@ func sameFile(x, y string) bool { } func unparen(e ast.Expr) ast.Expr { return astutil.Unparen(e) } + +func is[T any](x any) bool { + _, ok := x.(T) + return ok +} diff --git a/refactor/satisfy/find.go b/refactor/satisfy/find.go index 612f0a8e0..bab0e3cfd 100644 --- a/refactor/satisfy/find.go +++ b/refactor/satisfy/find.go @@ -355,7 +355,7 @@ func (f *Finder) expr(e ast.Expr) types.Type { f.sig = saved case *ast.CompositeLit: - switch T := coreType(deref(tv.Type)).(type) { + switch T := coreType(typeparams.Deref(tv.Type)).(type) { case *types.Struct: for i, elem := range e.Elts { if kv, ok := elem.(*ast.KeyValueExpr); ok { @@ -690,7 +690,7 @@ func (f *Finder) stmt(s ast.Stmt) { case *types.Map: xelem = ux.Elem() case *types.Pointer: // *array - xelem = coreType(deref(ux)).(*types.Array).Elem() + xelem = coreType(typeparams.Deref(ux)).(*types.Array).Elem() case *types.Slice: xelem = ux.Elem() } @@ -708,14 +708,6 @@ func (f *Finder) stmt(s ast.Stmt) { // -- Plundered from golang.org/x/tools/go/ssa ----------------- -// deref returns a pointer's element type; otherwise it returns typ. -func deref(typ types.Type) types.Type { - if p, ok := coreType(typ).(*types.Pointer); ok { - return p.Elem() - } - return typ -} - func unparen(e ast.Expr) ast.Expr { return astutil.Unparen(e) } func isInterface(T types.Type) bool { return types.IsInterface(T) }