From 51df92b224f5bbb7d2cabb52c0c77410b2d590e7 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Mon, 6 Nov 2023 13:27:49 -0500
Subject: [PATCH] go/ssa: two minor cleanups

1. inline sole uses of addMethod, createMethodSet.
2. simplify MethodValue call to LookupMethod in go/callgraph.

Change-Id: Ifa94d0dd7c2a522551c0d76341213dc9c8a73501
Reviewed-on: https://go-review.googlesource.com/c/tools/+/540216
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 go/callgraph/rta/rta.go |  2 +-
 go/callgraph/vta/vta.go |  3 ++
 go/ssa/methods.go       | 84 ++++++++++++++++-------------------------
 3 files changed, 37 insertions(+), 52 deletions(-)

diff --git a/go/callgraph/rta/rta.go b/go/callgraph/rta/rta.go
index 36fe93f6056..d0ae0fccf57 100644
--- a/go/callgraph/rta/rta.go
+++ b/go/callgraph/rta/rta.go
@@ -232,7 +232,7 @@ func (r *rta) visitDynCall(site ssa.CallInstruction) {
 func (r *rta) addInvokeEdge(site ssa.CallInstruction, C types.Type) {
 	// Ascertain the concrete method of C to be called.
 	imethod := site.Common().Method
-	cmethod := r.prog.MethodValue(r.prog.MethodSets.MethodSet(C).Lookup(imethod.Pkg(), imethod.Name()))
+	cmethod := r.prog.LookupMethod(C, imethod.Pkg(), imethod.Name())
 	r.addEdge(site.Parent(), site, cmethod, true)
 }
 
diff --git a/go/callgraph/vta/vta.go b/go/callgraph/vta/vta.go
index 58393600337..2303fcfa0a8 100644
--- a/go/callgraph/vta/vta.go
+++ b/go/callgraph/vta/vta.go
@@ -154,6 +154,9 @@ func propFunc(p propType, c ssa.CallInstruction, cache methodCache) []*ssa.Funct
 // ssa.Program.MethodSets and ssa.Program.MethodValue
 // APIs. The cache is used to speed up querying of
 // methods of a type as the mentioned APIs are expensive.
+//
+// TODO(adonovan): Program.MethodValue already does this kind of
+// caching. Is this really necessary?
 type methodCache map[types.Type]map[string][]*ssa.Function
 
 // methods returns methods of a type `t` named `name`. First consults
diff --git a/go/ssa/methods.go b/go/ssa/methods.go
index 2700d26a72c..03ef62521d9 100644
--- a/go/ssa/methods.go
+++ b/go/ssa/methods.go
@@ -42,9 +42,39 @@ func (prog *Program) MethodValue(sel *types.Selection) *Function {
 
 	var cr creator
 
-	prog.methodsMu.Lock()
-	m := prog.addMethod(prog.createMethodSet(T), sel, &cr)
-	prog.methodsMu.Unlock()
+	m := func() *Function {
+		prog.methodsMu.Lock()
+		defer prog.methodsMu.Unlock()
+
+		// Get or create SSA method set.
+		mset, ok := prog.methodSets.At(T).(*methodSet)
+		if !ok {
+			mset = &methodSet{mapping: make(map[string]*Function)}
+			prog.methodSets.Set(T, mset)
+		}
+
+		// Get or create SSA method.
+		id := sel.Obj().Id()
+		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
+			if needsPromotion || needsIndirection {
+				fn = createWrapper(prog, toSelection(sel), &cr)
+			} else {
+				fn = prog.objectMethod(obj, &cr)
+			}
+			if fn.Signature.Recv() == nil {
+				panic(fn)
+			}
+			mset.mapping[id] = fn
+		}
+
+		return fn
+	}()
 
 	b := builder{created: &cr}
 	b.iterate()
@@ -111,54 +141,6 @@ type methodSet struct {
 	mapping map[string]*Function // populated lazily
 }
 
-// Precondition: T is a concrete type, e.g. !isInterface(T) and not parameterized.
-// Requires prog.methodsMu.
-func (prog *Program) createMethodSet(T types.Type) *methodSet {
-	if prog.mode&SanityCheckFunctions != 0 {
-		if types.IsInterface(T) || prog.parameterized.isParameterized(T) {
-			panic("type is interface or parameterized")
-		}
-	}
-
-	mset, ok := prog.methodSets.At(T).(*methodSet)
-	if !ok {
-		mset = &methodSet{mapping: make(map[string]*Function)}
-		prog.methodSets.Set(T, mset)
-	}
-	return mset
-}
-
-// Adds any created functions to cr.
-// Precondition: T is a concrete type, e.g. !isInterface(T) and not parameterized.
-// Requires prog.methodsMu.
-func (prog *Program) addMethod(mset *methodSet, sel *types.Selection, cr *creator) *Function {
-	if sel.Kind() == types.MethodExpr {
-		panic(sel)
-	}
-	id := sel.Obj().Id()
-	fn := mset.mapping[id]
-	if fn == nil {
-		sel := toSelection(sel)
-		obj := sel.obj.(*types.Func)
-
-		_, ptrObj := deptr(recvType(obj))
-		_, ptrRecv := deptr(sel.recv)
-
-		needsPromotion := len(sel.index) > 1
-		needsIndirection := !ptrObj && ptrRecv
-		if needsPromotion || needsIndirection {
-			fn = createWrapper(prog, sel, cr)
-		} else {
-			fn = prog.objectMethod(obj, cr)
-		}
-		if fn.Signature.Recv() == nil {
-			panic(fn) // missing receiver
-		}
-		mset.mapping[id] = fn
-	}
-	return fn
-}
-
 // RuntimeTypes returns a new unordered slice containing all types in
 // the program for which a runtime type is required.
 //