internal/typesinternal: avoid linkname hacks This CL inverts the import relationship between typesinternal and the public typeutil package. Now: typeutil -> typesinternal. The implementations of Callee and StaticCallee were moved down into typesinternal, and the linkname hack (a recipe for trouble) was removed. The typesinternal.ForEachElement function was simplified by combining the 'yield' and 'de-dup' roles into a single parameter, breaking the dependency on typeutil.TypeMap; and the dependency on MethodSetCache was replaced by a closure for the necessary method. Also, in three places, Uses[UsedIdent()] was changed to do an explicit nil check to save a doomed map lookup. The only remaining use of linkname in x/tools is in go/ssa. Change-Id: Iae12162ad393d41e174bbbddb5fe5a3c142eab2f Reviewed-on: https://go-review.googlesource.com/c/tools/+/798881 LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alex Putman <aputman@golang.org> Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/go/ssa/methods.go b/go/ssa/methods.go index 82faade..666ef36 100644 --- a/go/ssa/methods.go +++ b/go/ssa/methods.go
@@ -167,10 +167,15 @@ // eliminates the need to eagerly compute all the element // types during SSA building. var runtimeTypes []types.Type - add := func(t types.Type) { runtimeTypes = append(runtimeTypes, t) } var set typeutil.Map // for de-duping identical types for t := range prog.makeInterfaceTypes { - typesinternal.ForEachElement(&set, &prog.MethodSets, t, add) + typesinternal.ForEachElement(prog.MethodSets.MethodSet, t, func(t types.Type) bool { + seen, _ := set.Set(t, true).(bool) + if !seen { + runtimeTypes = append(runtimeTypes, t) + } + return seen + }) } return runtimeTypes
diff --git a/go/types/typeutil/callee.go b/go/types/typeutil/callee.go index 3d24a8c..b64a8f4 100644 --- a/go/types/typeutil/callee.go +++ b/go/types/typeutil/callee.go
@@ -7,7 +7,8 @@ import ( "go/ast" "go/types" - _ "unsafe" // for linkname + + "golang.org/x/tools/internal/typesinternal" ) // Callee returns the named target of a function call, if any: @@ -19,14 +20,7 @@ // Note: for calls of instantiated functions and methods, Callee returns // the corresponding generic function or method on the generic type. func Callee(info *types.Info, call *ast.CallExpr) types.Object { - obj := info.Uses[usedIdent(info, call.Fun)] - if obj == nil { - return nil - } - if _, ok := obj.(*types.TypeName); ok { - return nil - } - return obj + return typesinternal.Callee(info, call) } // StaticCallee returns the target (function or method) of a static function @@ -35,52 +29,5 @@ // Note: for calls of instantiated functions and methods, StaticCallee returns // the corresponding generic function or method on the generic type. func StaticCallee(info *types.Info, call *ast.CallExpr) *types.Func { - obj := info.Uses[usedIdent(info, call.Fun)] - fn, _ := obj.(*types.Func) - if fn == nil || interfaceMethod(fn) { - return nil - } - return fn -} - -// usedIdent is the implementation of [internal/typesinternal.UsedIdent]. -// It returns the identifier associated with e. -// See typesinternal.UsedIdent for a fuller description. -// This function should live in typesinternal, but cannot because it would -// create an import cycle. -// -//go:linkname usedIdent golang.org/x/tools/go/types/typeutil.usedIdent -func usedIdent(info *types.Info, e ast.Expr) *ast.Ident { - if info.Types == nil || info.Uses == nil { - panic("one of info.Types or info.Uses is nil; both must be populated") - } - // Look through type instantiation if necessary. - switch d := ast.Unparen(e).(type) { - case *ast.IndexExpr: - if info.Types[d.Index].IsType() { - e = d.X - } - case *ast.IndexListExpr: - e = d.X - } - - switch e := ast.Unparen(e).(type) { - // info.Uses always has the object we want, even for selector expressions. - // We don't need info.Selections. - // See go/types/recording.go:recordSelection. - case *ast.Ident: - return e - case *ast.SelectorExpr: - return e.Sel - } - return nil -} - -// interfaceMethod reports whether its argument is a method of an interface. -// This function should live in typesinternal, but cannot because it would create an import cycle. -// -//go:linkname interfaceMethod golang.org/x/tools/go/types/typeutil.interfaceMethod -func interfaceMethod(f *types.Func) bool { - recv := f.Signature().Recv() - return recv != nil && types.IsInterface(recv.Type()) + return typesinternal.StaticCallee(info, call) }
diff --git a/internal/typesinternal/classify_call.go b/internal/typesinternal/classify_call.go index 7ebe976..14f616f 100644 --- a/internal/typesinternal/classify_call.go +++ b/internal/typesinternal/classify_call.go
@@ -8,7 +8,6 @@ "fmt" "go/ast" "go/types" - _ "unsafe" // for go:linkname hack ) // CallKind describes the function position of an [*ast.CallExpr]. @@ -72,11 +71,15 @@ if tv.IsBuiltin() { return CallBuiltin } - obj := info.Uses[UsedIdent(info, call.Fun)] + id := UsedIdent(info, call.Fun) + if id == nil { + return CallDynamic + } + obj := info.Uses[id] // Classify the call by the type of the object, if any. switch obj := obj.(type) { case *types.Func: - if interfaceMethod(obj) { + if isInterfaceMethod(obj) { return CallInterface } return CallStatic @@ -127,11 +130,66 @@ // Note: if e is an instantiated function or method, UsedIdent returns // the corresponding generic function or method on the generic type. func UsedIdent(info *types.Info, e ast.Expr) *ast.Ident { - return usedIdent(info, e) + if info.Types == nil || info.Uses == nil { + panic("one of info.Types or info.Uses is nil; both must be populated") + } + // Look through type instantiation if necessary. + switch d := ast.Unparen(e).(type) { + case *ast.IndexExpr: + if info.Types[d.Index].IsType() { + e = d.X + } + case *ast.IndexListExpr: + e = d.X + } + + switch e := ast.Unparen(e).(type) { + // info.Uses always has the object we want, even for selector expressions. + // We don't need info.Selections. + // See go/types/recording.go:recordSelection. + case *ast.Ident: + return e + case *ast.SelectorExpr: + return e.Sel + } + return nil } -//go:linkname usedIdent golang.org/x/tools/go/types/typeutil.usedIdent -func usedIdent(info *types.Info, e ast.Expr) *ast.Ident +// See [golang.org/x/tools/go/types/typeutil.Callee]. +func Callee(info *types.Info, call *ast.CallExpr) types.Object { + id := UsedIdent(info, call.Fun) + if id == nil { + return nil + } + obj := info.Uses[id] + if obj == nil { + return nil + } + if _, ok := obj.(*types.TypeName); ok { + return nil + } + return obj +} -//go:linkname interfaceMethod golang.org/x/tools/go/types/typeutil.interfaceMethod -func interfaceMethod(f *types.Func) bool +// See [golang.org/x/tools/go/types/typeutil.StaticCallee]. +func StaticCallee(info *types.Info, call *ast.CallExpr) *types.Func { + id := UsedIdent(info, call.Fun) + if id == nil { + return nil + } + obj := info.Uses[id] + if obj == nil { + return nil + } + fn, _ := obj.(*types.Func) + if fn == nil || isInterfaceMethod(fn) { + return nil + } + return fn +} + +// isInterfaceMethod reports whether its argument is a method of an interface. +func isInterfaceMethod(f *types.Func) bool { + recv := f.Signature().Recv() + return recv != nil && types.IsInterface(recv.Type()) +}
diff --git a/internal/typesinternal/element.go b/internal/typesinternal/element.go index 89eeea1..6e0613c 100644 --- a/internal/typesinternal/element.go +++ b/internal/typesinternal/element.go
@@ -7,8 +7,6 @@ import ( "fmt" "go/types" - - "golang.org/x/tools/go/types/typeutil" ) // ForEachElement calls f for type T and each type reachable from its @@ -16,25 +14,25 @@ // type constructors; in addition, for each named type N, the type *N // is added to the result as it may have additional methods. // -// The caller must provide an initially empty set used to de-duplicate -// identical types, potentially across multiple calls to ForEachElement. -// (Its final value holds all the elements seen, matching the arguments -// passed to f.) +// The result of f indicates whether the caller has seen this type +// already, so we can prune the traversal. // // TODO(adonovan): share/harmonize with go/callgraph/rta. -func ForEachElement(rtypes *typeutil.Map, msets *typeutil.MethodSetCache, T types.Type, f func(types.Type)) { +// +// methodSetOf abstracts (*typeutil.MethodSetCache).MethodSet, +// avoiding an import cycle. +func ForEachElement(methodSetOf func(types.Type) *types.MethodSet, T types.Type, f func(types.Type) bool) { var visit func(T types.Type, skip bool) visit = func(T types.Type, skip bool) { if !skip { - if seen, _ := rtypes.Set(T, true).(bool); seen { - return // de-dup + // notify caller of element type + if f(T) { + return // duplicate; prune descent } - - f(T) // notify caller of new element type } // Recursion over signatures of each method. - tmset := msets.MethodSet(T) + tmset := methodSetOf(T) for method := range tmset.Methods() { sig := method.Type().(*types.Signature) if sig.TypeParams() != nil {
diff --git a/internal/typesinternal/element_test.go b/internal/typesinternal/element_test.go index 7e70d0b..6078936 100644 --- a/internal/typesinternal/element_test.go +++ b/internal/typesinternal/element_test.go
@@ -135,26 +135,15 @@ } got := make(map[string]bool) - set := new(typeutil.Map) // for de-duping - set2 := new(typeutil.Map) // for consistency check - typesinternal.ForEachElement(set, &msets, T, func(elem types.Type) { - got[toStr(elem)] = true - set2.Set(elem, true) + set := new(typeutil.Map) // for de-duping + typesinternal.ForEachElement(msets.MethodSet, T, func(T types.Type) bool { + seen, _ := set.Set(T, true).(bool) + if !seen { + got[toStr(T)] = true + } + return seen }) - // Assert that set==set2, meaning f(x) was - // called for each x in the de-duping map. - if set.Len() != set2.Len() { - t.Errorf("ForEachElement called f %d times yet de-dup set has %d elements", - set2.Len(), set.Len()) - } else { - set.Iterate(func(key types.Type, _ any) { - if set2.At(key) == nil { - t.Errorf("ForEachElement did not call f(%v)", key) - } - }) - } - // Assert than all expected (and no unexpected) elements were found. fail := false for _, typstr := range test.want {