WIP: Use MethodByName only with constants

We would like to enable dead-code-elimination, as unblocked by https://go-review.googlesource.com/c/go/+/522436

Author: justinsb

upup/pkg/fi/cloudup/gcetasks/subnet.go

@@ -299,7 +299,7 @@ type terraformSubnetRange struct {
 	CIDR string `cty:"ip_cidr_range"`
 }
 
-func (_ *Subnet) RenderSubnet(t *terraform.TerraformTarget, a, e, changes *Subnet) error {
+func (_ *Subnet) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *Subnet) error {
 	shared := fi.ValueOf(e.Shared)
 	if shared {
 		// Not terraform owned / managed

upup/pkg/fi/context.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/nodeup"
+	"k8s.io/kops/util/pkg/reflectutils"
 	"k8s.io/kops/util/pkg/vfs"
 )
 
@@ -136,7 +137,7 @@ func (c *Context[T]) RunTasks(options RunTasksOptions) error {
 // it is typically called after we have checked the existing state of the Task and determined that is different
 // from the desired state.
 func (c *Context[T]) Render(a, e, changes Task[T]) error {
-	typeContextPtr := reflect.TypeOf((*Context[T])(nil))
+	// typeContextPtr := reflect.TypeOf((*Context[T])(nil))
 	var lifecycle Lifecycle
 	if hl, ok := e.(HasLifecycle); ok {
 		lifecycle = hl.GetLifecycle()
@@ -192,60 +193,73 @@ func (c *Context[T]) Render(a, e, changes Task[T]) error {
 	}
 
 	v := reflect.ValueOf(e)
-	vType := v.Type()
+	// vType := v.Type()
 
-	targetType := reflect.ValueOf(c.Target).Type()
+	// targetType := reflect.ValueOf(c.Target).Type()
 
-	var renderer *reflect.Method
+	var renderer *reflect.Value
 	var rendererArgs []reflect.Value
+	rendererName := ""
 
-	for i := 0; i < vType.NumMethod(); i++ {
-		method := vType.Method(i)
-		if !strings.HasPrefix(method.Name, "Render") {
+	renderMethodNames := []string{"Render"}
+
+	targetTypeName := fmt.Sprintf("%T", c.Target)
+	switch targetTypeName {
+	case "Foo":
+
+	default:
+		panic(fmt.Sprintf("targetType %q is not recognized", targetTypeName))
+	}
+	for _, methodName := range renderMethodNames {
+		method := reflectutils.GetMethodByName(v, methodName)
+		if method.IsZero() {
 			continue
 		}
 		match := true
 
-		var args []reflect.Value
-		for j := 0; j < method.Type.NumIn(); j++ {
-			arg := method.Type.In(j)
-			if arg.ConvertibleTo(vType) {
-				continue
-			}
-			if arg.ConvertibleTo(typeContextPtr) {
-				args = append(args, reflect.ValueOf(c))
-				continue
-			}
-			if arg.ConvertibleTo(targetType) {
-				args = append(args, reflect.ValueOf(c.Target))
-				continue
-			}
-			match = false
-			break
-		}
+		// var args []reflect.Value
+		// for j := 0; j < method.Type.NumIn(); j++ {
+		// 	arg := method.Type.In(j)
+		// 	if arg.ConvertibleTo(vType) {
+		// 		continue
+		// 	}
+		// 	if arg.ConvertibleTo(typeContextPtr) {
+		// 		args = append(args, reflect.ValueOf(c))
+		// 		continue
+		// 	}
+		// 	if arg.ConvertibleTo(targetType) {
+		// 		args = append(args, reflect.ValueOf(c.Target))
+		// 		continue
+		// 	}
+		// 	match = false
+		// 	break
+		// }
 		if match {
 			if renderer != nil {
-				if method.Name == "Render" {
+				if methodName == "Render" {
 					continue
 				}
-				if renderer.Name != "Render" {
+				if rendererName != "Render" {
 					return fmt.Errorf("found multiple Render methods that could be involved on %T", e)
 				}
 			}
 			renderer = &method
-			rendererArgs = args
+			rendererName = methodName
+			// rendererArgs = args
 		}
 
 	}
 	if renderer == nil {
 		return fmt.Errorf("could not find Render method on type %T (target %T)", e, c.Target)
 	}
+
+	rendererArgs = append(rendererArgs, reflect.ValueOf(c))
 	rendererArgs = append(rendererArgs, reflect.ValueOf(a))
 	rendererArgs = append(rendererArgs, reflect.ValueOf(e))
 	rendererArgs = append(rendererArgs, reflect.ValueOf(changes))
-	klog.V(11).Infof("Calling method %s on %T", renderer.Name, e)
-	m := v.MethodByName(renderer.Name)
-	rv := m.Call(rendererArgs)
+	klog.V(11).Infof("Calling method %s on %T", rendererName, e)
+	// m := v.MethodByName(renderer.Name)
+	rv := (*renderer).Call(rendererArgs)
 	var rvErr error
 	if !rv[0].IsNil() {
 		rvErr = rv[0].Interface().(error)

util/pkg/reflectutils/walk.go

@@ -57,14 +57,29 @@ func JSONMergeStruct(dest, src interface{}) {
 	}
 }
 
+// GetMethodByName implements a switch statement so that we can help the compiler with dead-code-elimination.
+// For background: https://github.com/golang/protobuf/issues/1561
+func GetMethodByName(reflectValue reflect.Value, methodName string) reflect.Value {
+	switch methodName {
+	case "CheckChanges":
+		return reflectValue.MethodByName("CheckChanges")
+	case "Find":
+		return reflectValue.MethodByName("Find")
+	case "ShouldCreate":
+		return reflectValue.MethodByName("ShouldCreate")
+	default:
+		panic(fmt.Sprintf("need to add %q to getMethodByName helper", methodName))
+	}
+}
+
 // InvokeMethod calls the specified method by reflection
-func InvokeMethod(target interface{}, name string, args ...interface{}) ([]reflect.Value, error) {
+func InvokeMethod(target interface{}, methodName string, args ...interface{}) ([]reflect.Value, error) {
 	v := reflect.ValueOf(target)
 
-	method, found := v.Type().MethodByName(name)
-	if !found {
+	m := GetMethodByName(v, methodName)
+	if m.IsZero() {
 		return nil, &MethodNotFoundError{
-			Name:   name,
+			Name:   methodName,
 			Target: target,
 		}
 	}
@@ -73,8 +88,7 @@ func InvokeMethod(target interface{}, name string, args ...interface{}) ([]refle
 	for _, a := range args {
 		argValues = append(argValues, reflect.ValueOf(a))
 	}
-	klog.V(12).Infof("Calling method %s on %T", method.Name, target)
-	m := v.MethodByName(method.Name)
+	klog.V(12).Infof("Calling method %s on %T", methodName, target)
 	rv := m.Call(argValues)
 	return rv, nil
 }