go/ssa: extract type recursion into a helper package

This change moves ssa.forEachReachable into
internal/typesinternal.ForEachElement, simplifies
the signature (by internalizing the type map part)
and adds a test.

There are two copies of this algorithm (the other in
go/callgraph/rta), and we will need it again in
ssautil.Reachable (see golang/go#69291).
A follow-up change will make the copy in rta delegate
to this one (small steps).

Also, make ssa.Program.RuntimeTypes do the type analysis
when it is called, instead of doing it eagerly each time
we encounter a MakeInterface instruction.
This should reduce eventually costs since RuntimeTypes
shouldn't be needed: it was added for the pointer analysis
(since deleted) and it used by ssautil.AllFunctions (to
be replaced, see golang/go#69291), and in both cases it
is the wrong tool for the job because:

(a) it is more precise to accumulate runtime types in
    the subset of the program of interest, while doing
    some kind of reachability fixed-point computation;
    and

(b) its use in AllFunctions is unsound because although
    it accounts for all (too many!) MakeInterface operations
    it does not account for types exposed through public
    API (see the proposed replacement, ssautil.Reachable)
    when analyzing incomplete programs.

Updates golang/go#69291

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

Author: adonovan

go/ssa/builder.go

@@ -3104,17 +3104,17 @@ func (b *builder) buildYieldFunc(fn *Function) {
 	fn.finishBody()
 }
 
-// addRuntimeType records t as a runtime type,
-// along with all types derivable from it using reflection.
+// addMakeInterfaceType records non-interface type t as the type of
+// the operand a MakeInterface operation, for [Program.RuntimeTypes].
 //
-// Acquires prog.runtimeTypesMu.
-func addRuntimeType(prog *Program, t types.Type) {
-	prog.runtimeTypesMu.Lock()
-	defer prog.runtimeTypesMu.Unlock()
-	forEachReachable(&prog.MethodSets, t, func(t types.Type) bool {
-		prev, _ := prog.runtimeTypes.Set(t, true).(bool)
-		return !prev // already seen?
-	})
+// Acquires prog.makeInterfaceTypesMu.
+func addMakeInterfaceType(prog *Program, t types.Type) {
+	prog.makeInterfaceTypesMu.Lock()
+	defer prog.makeInterfaceTypesMu.Unlock()
+	if prog.makeInterfaceTypes == nil {
+		prog.makeInterfaceTypes = make(map[types.Type]unit)
+	}
+	prog.makeInterfaceTypes[t] = unit{}
 }
 
 // Build calls Package.Build for each package in prog.

go/ssa/emit.go

@@ -249,7 +249,7 @@ func emitConv(f *Function, val Value, typ types.Type) Value {
 		// non-parameterized, as they are the set of runtime types.
 		t := val.Type()
 		if f.typeparams.Len() == 0 || !f.Prog.isParameterized(t) {
-			addRuntimeType(f.Prog, t)
+			addMakeInterfaceType(f.Prog, t)
 		}
 
 		mi := &MakeInterface{X: val}

go/ssa/methods.go

@@ -11,7 +11,7 @@ import (
 	"go/types"
 
 	"golang.org/x/tools/go/types/typeutil"
-	"golang.org/x/tools/internal/aliases"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 // MethodValue returns the Function implementing method sel, building
@@ -158,124 +158,23 @@ type methodSet struct {
 //
 // Thread-safe.
 //
-// Acquires prog.runtimeTypesMu.
+// Acquires prog.makeInterfaceTypesMu.
 func (prog *Program) RuntimeTypes() []types.Type {
-	prog.runtimeTypesMu.Lock()
-	defer prog.runtimeTypesMu.Unlock()
-	return prog.runtimeTypes.Keys()
-}
-
-// forEachReachable calls f for type T and each type reachable from
-// its type through reflection.
-//
-// The function f must use memoization to break cycles and
-// return false when the type has already been visited.
-//
-// TODO(adonovan): publish in typeutil and share with go/callgraph/rta.
-func forEachReachable(msets *typeutil.MethodSetCache, 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 !f(T) {
-				return
-			}
-		}
-
-		// Recursion over signatures of each method.
-		tmset := msets.MethodSet(T)
-		for i := 0; i < tmset.Len(); i++ {
-			sig := tmset.At(i).Type().(*types.Signature)
-			// It is tempting to call visit(sig, false)
-			// but, as noted in golang.org/cl/65450043,
-			// the Signature.Recv field is ignored by
-			// types.Identical and typeutil.Map, which
-			// is confusing at best.
-			//
-			// More importantly, the true signature rtype
-			// reachable from a method using reflection
-			// has no receiver but an extra ordinary parameter.
-			// For the Read method of io.Reader we want:
-			//   func(Reader, []byte) (int, error)
-			// but here sig is:
-			//   func([]byte) (int, error)
-			// with .Recv = Reader (though it is hard to
-			// notice because it doesn't affect Signature.String
-			// or types.Identical).
-			//
-			// TODO(adonovan): construct and visit the correct
-			// non-method signature with an extra parameter
-			// (though since unnamed func types have no methods
-			// there is essentially no actual demand for this).
-			//
-			// TODO(adonovan): document whether or not it is
-			// safe to skip non-exported methods (as RTA does).
-			visit(sig.Params(), true)  // skip the Tuple
-			visit(sig.Results(), true) // skip the Tuple
-		}
-
-		switch T := T.(type) {
-		case *aliases.Alias:
-			visit(aliases.Unalias(T), skip) // emulates the pre-Alias behavior
-
-		case *types.Basic:
-			// nop
-
-		case *types.Interface:
-			// nop---handled by recursion over method set.
-
-		case *types.Pointer:
-			visit(T.Elem(), false)
-
-		case *types.Slice:
-			visit(T.Elem(), false)
-
-		case *types.Chan:
-			visit(T.Elem(), false)
-
-		case *types.Map:
-			visit(T.Key(), false)
-			visit(T.Elem(), false)
-
-		case *types.Signature:
-			if T.Recv() != nil {
-				panic(fmt.Sprintf("Signature %s has Recv %s", T, T.Recv()))
-			}
-			visit(T.Params(), true)  // skip the Tuple
-			visit(T.Results(), true) // skip the Tuple
-
-		case *types.Named:
-			// A pointer-to-named type can be derived from a named
-			// type via reflection.  It may have methods too.
-			visit(types.NewPointer(T), false)
-
-			// Consider 'type T struct{S}' where S has methods.
-			// Reflection provides no way to get from T to struct{S},
-			// only to S, so the method set of struct{S} is unwanted,
-			// so set 'skip' flag during recursion.
-			visit(T.Underlying(), true) // skip the unnamed type
-
-		case *types.Array:
-			visit(T.Elem(), false)
-
-		case *types.Struct:
-			for i, n := 0, T.NumFields(); i < n; i++ {
-				// TODO(adonovan): document whether or not
-				// it is safe to skip non-exported fields.
-				visit(T.Field(i).Type(), false)
-			}
-
-		case *types.Tuple:
-			for i, n := 0, T.Len(); i < n; i++ {
-				visit(T.At(i).Type(), false)
-			}
-
-		case *types.TypeParam, *types.Union:
-			// forEachReachable must not be called on parameterized types.
-			panic(T)
-
-		default:
-			panic(T)
-		}
+	prog.makeInterfaceTypesMu.Lock()
+	defer prog.makeInterfaceTypesMu.Unlock()
+
+	// Compute the derived types on demand, since many SSA clients
+	// never call RuntimeTypes, and those that do typically call
+	// it once (often within ssautil.AllFunctions, which will
+	// eventually not use it; see Go issue #69291.) This
+	// 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)
 	}
-	visit(T, false)
+
+	return runtimeTypes
 }

go/ssa/ssa.go

@@ -37,8 +37,9 @@ type Program struct {
 	hasParamsMu sync.Mutex
 	hasParams   typeparams.Free
 
-	runtimeTypesMu sync.Mutex
-	runtimeTypes   typeutil.Map // set of runtime types (from MakeInterface)
+	// set of concrete types used as MakeInterface operands
+	makeInterfaceTypesMu sync.Mutex
+	makeInterfaceTypes   map[types.Type]unit // (may contain redundant identical types)
 
 	// objectMethods is a memoization of objectMethod
 	// to avoid creation of duplicate methods from type information.

internal/typesinternal/element.go

@@ -0,0 +1,134 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package typesinternal
+
+import (
+	"fmt"
+	"go/types"
+
+	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/aliases"
+)
+
+// ForEachElement calls f for type T and each type reachable from its
+// type through reflection. It does this by recursively stripping off
+// 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.)
+//
+// TODO(adonovan): share/harmonize with go/callgraph/rta.
+func ForEachElement(rtypes *typeutil.Map, msets *typeutil.MethodSetCache, T types.Type, f func(types.Type)) {
+	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
+			}
+
+			f(T) // notify caller of new element type
+		}
+
+		// Recursion over signatures of each method.
+		tmset := msets.MethodSet(T)
+		for i := 0; i < tmset.Len(); i++ {
+			sig := tmset.At(i).Type().(*types.Signature)
+			// It is tempting to call visit(sig, false)
+			// but, as noted in golang.org/cl/65450043,
+			// the Signature.Recv field is ignored by
+			// types.Identical and typeutil.Map, which
+			// is confusing at best.
+			//
+			// More importantly, the true signature rtype
+			// reachable from a method using reflection
+			// has no receiver but an extra ordinary parameter.
+			// For the Read method of io.Reader we want:
+			//   func(Reader, []byte) (int, error)
+			// but here sig is:
+			//   func([]byte) (int, error)
+			// with .Recv = Reader (though it is hard to
+			// notice because it doesn't affect Signature.String
+			// or types.Identical).
+			//
+			// TODO(adonovan): construct and visit the correct
+			// non-method signature with an extra parameter
+			// (though since unnamed func types have no methods
+			// there is essentially no actual demand for this).
+			//
+			// TODO(adonovan): document whether or not it is
+			// safe to skip non-exported methods (as RTA does).
+			visit(sig.Params(), true)  // skip the Tuple
+			visit(sig.Results(), true) // skip the Tuple
+		}
+
+		switch T := T.(type) {
+		case *aliases.Alias:
+			visit(aliases.Unalias(T), skip) // emulates the pre-Alias behavior
+
+		case *types.Basic:
+			// nop
+
+		case *types.Interface:
+			// nop---handled by recursion over method set.
+
+		case *types.Pointer:
+			visit(T.Elem(), false)
+
+		case *types.Slice:
+			visit(T.Elem(), false)
+
+		case *types.Chan:
+			visit(T.Elem(), false)
+
+		case *types.Map:
+			visit(T.Key(), false)
+			visit(T.Elem(), false)
+
+		case *types.Signature:
+			if T.Recv() != nil {
+				panic(fmt.Sprintf("Signature %s has Recv %s", T, T.Recv()))
+			}
+			visit(T.Params(), true)  // skip the Tuple
+			visit(T.Results(), true) // skip the Tuple
+
+		case *types.Named:
+			// A pointer-to-named type can be derived from a named
+			// type via reflection.  It may have methods too.
+			visit(types.NewPointer(T), false)
+
+			// Consider 'type T struct{S}' where S has methods.
+			// Reflection provides no way to get from T to struct{S},
+			// only to S, so the method set of struct{S} is unwanted,
+			// so set 'skip' flag during recursion.
+			visit(T.Underlying(), true) // skip the unnamed type
+
+		case *types.Array:
+			visit(T.Elem(), false)
+
+		case *types.Struct:
+			for i, n := 0, T.NumFields(); i < n; i++ {
+				// TODO(adonovan): document whether or not
+				// it is safe to skip non-exported fields.
+				visit(T.Field(i).Type(), false)
+			}
+
+		case *types.Tuple:
+			for i, n := 0, T.Len(); i < n; i++ {
+				visit(T.At(i).Type(), false)
+			}
+
+		case *types.TypeParam, *types.Union:
+			// forEachReachable must not be called on parameterized types.
+			panic(T)
+
+		default:
+			panic(T)
+		}
+	}
+	visit(T, false)
+}