gopls: rationalize "deref" helpers
This change (part of the types.Alias audit) defines two
helper functions for dealing with pointer types:
1. internal/typeparams.MustDeref (formerly go/ssa.mustDeref)
is the type equivalent of a LOAD instruction.
2. internal/typesinternal.Unpointer strips off a
Pointer constructor (possibly wrapped in an Alias).
The golang.Deref function, which recursively strips off
Pointer and Named constructors, is a meaningless
operation. It has been replaced in all cases by
Unpointer + Unalias.
There are far too many functions called 'deref' with
subtle variations in their behavior. I plan to
standardize x/tools on few common idioms.
(There is more to do.)
Updates golang/go#65294
Change-Id: I502bab95e8d954715784b7e35ec801f4be4bc959
Reviewed-on: https://go-review.googlesource.com/c/tools/+/565476
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Tim King <taking@google.com>
diff --git a/go/ssa/builder.go b/go/ssa/builder.go
index 8622dfc..ecbc3e4 100644
--- a/go/ssa/builder.go
+++ b/go/ssa/builder.go
@@ -328,7 +328,7 @@
}
case "new":
- return emitNew(fn, mustDeref(typ), pos, "new")
+ return emitNew(fn, typeparams.MustDeref(typ), pos, "new")
case "len", "cap":
// Special case: len or cap of an array or *array is
@@ -419,7 +419,7 @@
wantAddr := true
v := b.receiver(fn, e.X, wantAddr, escaping, sel)
index := sel.index[len(sel.index)-1]
- fld := fieldOf(mustDeref(v.Type()), index) // v is an addr.
+ fld := fieldOf(typeparams.MustDeref(v.Type()), index) // v is an addr.
// 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
@@ -468,7 +468,7 @@
v.setType(et)
return fn.emit(v)
}
- return &lazyAddress{addr: emit, t: mustDeref(et), pos: e.Lbrack, expr: e}
+ return &lazyAddress{addr: emit, t: typeparams.MustDeref(et), pos: e.Lbrack, expr: e}
case *ast.StarExpr:
return &address{addr: b.expr(fn, e.X), pos: e.Star, expr: e}
diff --git a/go/ssa/create.go b/go/ssa/create.go
index c4da35d..4545c17 100644
--- a/go/ssa/create.go
+++ b/go/ssa/create.go
@@ -259,6 +259,7 @@
obj := scope.Lookup(name)
memberFromObject(p, obj, nil, "")
if obj, ok := obj.(*types.TypeName); ok {
+ // No Unalias: aliases should not duplicate methods.
if named, ok := obj.Type().(*types.Named); ok {
for i, n := 0, named.NumMethods(); i < n; i++ {
memberFromObject(p, named.Method(i), nil, "")
diff --git a/go/ssa/emit.go b/go/ssa/emit.go
index d77b440..2a26c94 100644
--- a/go/ssa/emit.go
+++ b/go/ssa/emit.go
@@ -11,6 +11,8 @@
"go/ast"
"go/token"
"go/types"
+
+ "golang.org/x/tools/internal/typeparams"
)
// emitAlloc emits to f a new Alloc instruction allocating a variable
@@ -64,7 +66,7 @@
// new temporary, and returns the value so defined.
func emitLoad(f *Function, addr Value) *UnOp {
v := &UnOp{Op: token.MUL, X: addr}
- v.setType(mustDeref(addr.Type()))
+ v.setType(typeparams.MustDeref(addr.Type()))
f.emit(v)
return v
}
@@ -414,7 +416,7 @@
// emitStore emits to f an instruction to store value val at location
// addr, applying implicit conversions as required by assignability rules.
func emitStore(f *Function, addr, val Value, pos token.Pos) *Store {
- typ := mustDeref(addr.Type())
+ typ := typeparams.MustDeref(addr.Type())
s := &Store{
Addr: addr,
Val: emitConv(f, val, typ),
diff --git a/go/ssa/func.go b/go/ssa/func.go
index 0ac2204..4d3e391 100644
--- a/go/ssa/func.go
+++ b/go/ssa/func.go
@@ -14,6 +14,8 @@
"io"
"os"
"strings"
+
+ "golang.org/x/tools/internal/typeparams"
)
// Like ObjectOf, but panics instead of returning nil.
@@ -531,7 +533,7 @@
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(mustDeref(l.Type()), from))
+ fmt.Fprintf(buf, "# % 3d:\t%s %s\n", i, l.Name(), relType(typeparams.MustDeref(l.Type()), from))
}
}
writeSignature(buf, from, f.Name(), f.Signature)
diff --git a/go/ssa/lift.go b/go/ssa/lift.go
index da49fe9..8bb1949 100644
--- a/go/ssa/lift.go
+++ b/go/ssa/lift.go
@@ -43,6 +43,8 @@
"go/token"
"math/big"
"os"
+
+ "golang.org/x/tools/internal/typeparams"
)
// If true, show diagnostic information at each step of lifting.
@@ -465,7 +467,7 @@
*fresh++
phi.pos = alloc.Pos()
- phi.setType(mustDeref(alloc.Type()))
+ phi.setType(typeparams.MustDeref(alloc.Type()))
phi.block = v
if debugLifting {
fmt.Fprintf(os.Stderr, "\tplace %s = %s at block %s\n", phi.Name(), phi, v)
@@ -510,7 +512,7 @@
func renamed(renaming []Value, alloc *Alloc) Value {
v := renaming[alloc.index]
if v == nil {
- v = zeroConst(mustDeref(alloc.Type()))
+ v = zeroConst(typeparams.MustDeref(alloc.Type()))
renaming[alloc.index] = v
}
return v
diff --git a/go/ssa/lvalue.go b/go/ssa/lvalue.go
index 186cfca..eede307 100644
--- a/go/ssa/lvalue.go
+++ b/go/ssa/lvalue.go
@@ -11,6 +11,8 @@
"go/ast"
"go/token"
"go/types"
+
+ "golang.org/x/tools/internal/typeparams"
)
// An lvalue represents an assignable location that may appear on the
@@ -52,7 +54,7 @@
}
func (a *address) typ() types.Type {
- return mustDeref(a.addr.Type())
+ return typeparams.MustDeref(a.addr.Type())
}
// An element is an lvalue represented by m[k], the location of an
diff --git a/go/ssa/print.go b/go/ssa/print.go
index 727a735..38d8404 100644
--- a/go/ssa/print.go
+++ b/go/ssa/print.go
@@ -17,6 +17,7 @@
"strings"
"golang.org/x/tools/go/types/typeutil"
+ "golang.org/x/tools/internal/typeparams"
)
// relName returns the name of v relative to i.
@@ -94,7 +95,7 @@
op = "new"
}
from := v.Parent().relPkg()
- return fmt.Sprintf("%s %s (%s)", op, relType(mustDeref(v.Type()), from), v.Comment)
+ return fmt.Sprintf("%s %s (%s)", op, relType(typeparams.MustDeref(v.Type()), from), v.Comment)
}
func (v *Phi) String() string {
@@ -260,7 +261,7 @@
func (v *FieldAddr) String() string {
// Be robust against a bad index.
name := "?"
- if fld := fieldOf(mustDeref(v.X.Type()), v.Field); fld != nil {
+ if fld := fieldOf(typeparams.MustDeref(v.X.Type()), v.Field); fld != nil {
name = fld.Name()
}
return fmt.Sprintf("&%s.%s [#%d]", relName(v.X, v), name, v.Field)
@@ -449,7 +450,7 @@
case *Global:
fmt.Fprintf(buf, " var %-*s %s\n",
- maxname, name, relType(mustDeref(mem.Type()), from))
+ maxname, name, relType(typeparams.MustDeref(mem.Type()), from))
}
}
diff --git a/go/ssa/util.go b/go/ssa/util.go
index 729a4e5..915b4e2 100644
--- a/go/ssa/util.go
+++ b/go/ssa/util.go
@@ -115,15 +115,6 @@
return typ, false
}
-// mustDeref returns the element type of a type with a pointer core type.
-// Panics on failure.
-func mustDeref(typ types.Type) types.Type {
- if et, ok := deref(typ); ok {
- return et
- }
- panic("cannot dereference type " + typ.String())
-}
-
// recvType returns the receiver type of method obj.
func recvType(obj *types.Func) types.Type {
return obj.Type().(*types.Signature).Recv().Type()
diff --git a/gopls/internal/golang/completion/completion.go b/gopls/internal/golang/completion/completion.go
index b796de4..8ff7f0b 100644
--- a/gopls/internal/golang/completion/completion.go
+++ b/gopls/internal/golang/completion/completion.go
@@ -37,10 +37,12 @@
goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
"golang.org/x/tools/gopls/internal/util/safetoken"
"golang.org/x/tools/gopls/internal/util/typesutil"
+ "golang.org/x/tools/internal/aliases"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/fuzzy"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/typeparams"
+ "golang.org/x/tools/internal/typesinternal"
)
// A CompletionItem represents a possible completion suggested by the algorithm.
@@ -1585,7 +1587,7 @@
}
if c.inference.objType != nil {
- if named, _ := golang.Deref(c.inference.objType).(*types.Named); named != nil {
+ if named, ok := aliases.Unalias(typesinternal.Unpointer(c.inference.objType)).(*types.Named); ok {
// If we expected a named type, check the type's package for
// completion items. This is useful when the current file hasn't
// imported the type's package yet.
@@ -1651,14 +1653,14 @@
return
}
- t = golang.Deref(t)
+ t = typesinternal.Unpointer(t)
// If we have an expected type and it is _not_ a named type, handle
// it specially. Non-named types like "[]int" will never be
// considered via a lexical search, so we need to directly inject
// them. Also allow generic types since lexical search does not
// infer instantiated versions of them.
- if named, _ := t.(*types.Named); named == nil || named.TypeParams().Len() > 0 {
+ if named, ok := aliases.Unalias(t).(*types.Named); !ok || named.TypeParams().Len() > 0 {
// If our expected type is "[]int", this will add a literal
// candidate of "[]int{}".
c.literal(ctx, t, nil)
@@ -1896,7 +1898,7 @@
clInfo := compLitInfo{
cl: n,
- clType: golang.Deref(tv.Type).Underlying(),
+ clType: typesinternal.Unpointer(tv.Type).Underlying(),
}
var (
diff --git a/gopls/internal/golang/completion/literal.go b/gopls/internal/golang/completion/literal.go
index 45f772d..dc4fc0d 100644
--- a/gopls/internal/golang/completion/literal.go
+++ b/gopls/internal/golang/completion/literal.go
@@ -14,7 +14,9 @@
"golang.org/x/tools/gopls/internal/golang"
"golang.org/x/tools/gopls/internal/golang/completion/snippet"
"golang.org/x/tools/gopls/internal/protocol"
+ "golang.org/x/tools/internal/aliases"
"golang.org/x/tools/internal/event"
+ "golang.org/x/tools/internal/typesinternal"
)
// literal generates composite literal, function literal, and make()
@@ -49,10 +51,11 @@
//
// don't offer "mySlice{}" since we have already added a candidate
// of "[]int{}".
- if _, named := literalType.(*types.Named); named && expType != nil {
- if _, named := golang.Deref(expType).(*types.Named); !named {
- return
- }
+ if is[*types.Named](aliases.Unalias(literalType)) &&
+ expType != nil &&
+ !is[*types.Named](aliases.Unalias(typesinternal.Unpointer(expType))) {
+
+ return
}
// Check if an object of type literalType would match our expected type.
@@ -589,3 +592,8 @@
_, foundObj := scope.LookupParent(obj.Name(), c.pos)
return obj == foundObj
}
+
+func is[T any](x any) bool {
+ _, ok := x.(T)
+ return ok
+}
diff --git a/gopls/internal/golang/completion/postfix_snippets.go b/gopls/internal/golang/completion/postfix_snippets.go
index 252d2e7..fad8f78 100644
--- a/gopls/internal/golang/completion/postfix_snippets.go
+++ b/gopls/internal/golang/completion/postfix_snippets.go
@@ -21,8 +21,10 @@
"golang.org/x/tools/gopls/internal/golang/completion/snippet"
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/safetoken"
+ "golang.org/x/tools/internal/aliases"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
+ "golang.org/x/tools/internal/typesinternal"
)
// Postfix snippets are artificial methods that allow the user to
@@ -465,7 +467,7 @@
// go/types predicates are undefined on types.Typ[types.Invalid].
if !types.Identical(t, types.Typ[types.Invalid]) && types.Implements(t, errorIntf) {
name = "err"
- } else if _, isNamed := golang.Deref(t).(*types.Named); !isNamed {
+ } else if !is[*types.Named](aliases.Unalias(typesinternal.Unpointer(t))) {
name = nonNamedDefault
}
diff --git a/gopls/internal/golang/completion/util.go b/gopls/internal/golang/completion/util.go
index 65d1b4d..8ac7f16 100644
--- a/gopls/internal/golang/completion/util.go
+++ b/gopls/internal/golang/completion/util.go
@@ -14,6 +14,7 @@
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/util/safetoken"
"golang.org/x/tools/internal/diff"
+ "golang.org/x/tools/internal/typesinternal"
)
// exprAtPos returns the index of the expression containing pos.
@@ -38,7 +39,12 @@
var visit func(T types.Type)
visit = func(T types.Type) {
- if T, ok := golang.Deref(T).Underlying().(*types.Struct); ok {
+ // T may be a Struct, optionally Named, with an optional
+ // Pointer (with optional Aliases at every step!):
+ // Consider: type T *struct{ f int }; _ = T(nil).f
+ //
+ // TODO(adonovan): use typeparams.Deref in next CL.
+ if T, ok := typesinternal.Unpointer(T.Underlying()).Underlying().(*types.Struct); ok {
if seen.At(T) != nil {
return
}
diff --git a/gopls/internal/golang/hover.go b/gopls/internal/golang/hover.go
index 15bd97a..e7b903d 100644
--- a/gopls/internal/golang/hover.go
+++ b/gopls/internal/golang/hover.go
@@ -1226,7 +1226,7 @@
var fields []promotedField
var visit func(t types.Type, stack []*types.Named)
visit = func(t types.Type, stack []*types.Named) {
- tStruct, ok := Deref(t).Underlying().(*types.Struct)
+ tStruct, ok := typesinternal.Unpointer(t).Underlying().(*types.Struct)
if !ok {
return
}
diff --git a/gopls/internal/golang/identifier.go b/gopls/internal/golang/identifier.go
index 28f8975..994010f 100644
--- a/gopls/internal/golang/identifier.go
+++ b/gopls/internal/golang/identifier.go
@@ -8,6 +8,9 @@
"errors"
"go/ast"
"go/types"
+
+ "golang.org/x/tools/internal/aliases"
+ "golang.org/x/tools/internal/typesinternal"
)
// ErrNoIdentFound is error returned when no identifier is found at a particular position
@@ -23,24 +26,30 @@
return sig
}
+// searchForEnclosing returns, given the AST path to a SelectorExpr,
+// the exported named type of the innermost implicit field selection.
+//
+// For example, given "new(A).d" where this is (due to embedding) a
+// shorthand for "new(A).b.c.d", it returns the named type of c,
+// if it is exported, otherwise the type of b, or A.
func searchForEnclosing(info *types.Info, path []ast.Node) *types.TypeName {
for _, n := range path {
switch n := n.(type) {
case *ast.SelectorExpr:
if sel, ok := info.Selections[n]; ok {
- recv := Deref(sel.Recv())
+ recv := typesinternal.Unpointer(sel.Recv())
// Keep track of the last exported type seen.
var exported *types.TypeName
- if named, ok := recv.(*types.Named); ok && named.Obj().Exported() {
+ if named, ok := aliases.Unalias(recv).(*types.Named); ok && named.Obj().Exported() {
exported = named.Obj()
}
// We don't want the last element, as that's the field or
// method itself.
for _, index := range sel.Index()[:len(sel.Index())-1] {
if r, ok := recv.Underlying().(*types.Struct); ok {
- recv = Deref(r.Field(index).Type())
- if named, ok := recv.(*types.Named); ok && named.Obj().Exported() {
+ recv = typesinternal.Unpointer(r.Field(index).Type())
+ if named, ok := aliases.Unalias(recv).(*types.Named); ok && named.Obj().Exported() {
exported = named.Obj()
}
}
diff --git a/gopls/internal/golang/rename_check.go b/gopls/internal/golang/rename_check.go
index 11b154c..ffaff30 100644
--- a/gopls/internal/golang/rename_check.go
+++ b/gopls/internal/golang/rename_check.go
@@ -45,6 +45,8 @@
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/util/safetoken"
+ "golang.org/x/tools/internal/aliases"
+ "golang.org/x/tools/internal/typesinternal"
"golang.org/x/tools/refactor/satisfy"
)
@@ -371,7 +373,7 @@
return visit(nil) // pop stack, don't descend
}
// TODO(adonovan): fix: for generics, should be T.core not T.underlying.
- if _, ok := Deref(tv.Type).Underlying().(*types.Struct); ok {
+ if _, ok := typesinternal.Unpointer(tv.Type).Underlying().(*types.Struct); ok {
if n.Type != nil {
ast.Inspect(n.Type, visit)
}
@@ -501,7 +503,7 @@
if from.Anonymous() {
if named, ok := from.Type().(*types.Named); ok {
r.check(named.Obj())
- } else if named, ok := Deref(from.Type()).(*types.Named); ok {
+ } else if named, ok := aliases.Unalias(typesinternal.Unpointer(from.Type())).(*types.Named); ok {
r.check(named.Obj())
}
}
diff --git a/gopls/internal/golang/util.go b/gopls/internal/golang/util.go
index 5283a5b..c2f5d50 100644
--- a/gopls/internal/golang/util.go
+++ b/gopls/internal/golang/util.go
@@ -113,30 +113,6 @@
return FormatNode(fset, n)
}
-// Deref returns a pointer's element type, traversing as many levels as needed.
-// Otherwise it returns typ.
-//
-// It can return a pointer type for cyclic types (see golang/go#45510).
-func Deref(typ types.Type) types.Type {
- var seen map[types.Type]struct{}
- for {
- p, ok := typ.Underlying().(*types.Pointer)
- if !ok {
- return typ
- }
- if _, ok := seen[p.Elem()]; ok {
- return typ
- }
-
- typ = p.Elem()
-
- if seen == nil {
- seen = make(map[types.Type]struct{})
- }
- seen[typ] = struct{}{}
- }
-}
-
// findFileInDeps finds package metadata containing URI in the transitive
// dependencies of m. When using the Go command, the answer is unique.
func findFileInDeps(s metadata.Source, mp *metadata.Package, uri protocol.DocumentURI) *metadata.Package {
diff --git a/internal/typeparams/coretype.go b/internal/typeparams/coretype.go
index 7ea8840..048e216 100644
--- a/internal/typeparams/coretype.go
+++ b/internal/typeparams/coretype.go
@@ -5,6 +5,7 @@
package typeparams
import (
+ "fmt"
"go/types"
)
@@ -120,3 +121,15 @@
return []*types.Term{types.NewTerm(false, typ)}, nil
}
}
+
+// MustDeref returns the type of the variable pointed to by t.
+// It panics if t's core type is not a pointer.
+//
+// TODO(adonovan): ideally this would live in typesinternal, but that
+// creates an import cycle. Move there when we melt this package down.
+func MustDeref(t types.Type) types.Type {
+ if ptr, ok := CoreType(t).(*types.Pointer); ok {
+ return ptr.Elem()
+ }
+ panic(fmt.Sprintf("%v is not a pointer", t))
+}
diff --git a/internal/typesinternal/recv.go b/internal/typesinternal/recv.go
index 1a7b726..fea7c8b 100644
--- a/internal/typesinternal/recv.go
+++ b/internal/typesinternal/recv.go
@@ -22,3 +22,22 @@
named, _ = aliases.Unalias(t).(*types.Named)
return
}
+
+// Unpointer returns T given *T or an alias thereof.
+// For all other types it is the identity function.
+// It does not look at underlying types.
+// The result may be an alias.
+//
+// Use this function to strip off the optional pointer on a receiver
+// in a field or method selection, without losing the named type
+// (which is needed to compute the method set).
+//
+// See also [typeparams.MustDeref], which removes one level of
+// indirection from the type, regardless of named types (analogous to
+// a LOAD instruction).
+func Unpointer(t types.Type) types.Type {
+ if ptr, ok := aliases.Unalias(t).(*types.Pointer); ok {
+ return ptr.Elem()
+ }
+ return t
+}
