Trying to figure out how to encapsulate instance checking

Workiva · Sep 6, 2024 · 82e8d95 · 82e8d95
1 parent 23b0a38
commit 82e8d95
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 80 deletions.
diff --git a/compiler/expressions.go b/compiler/expressions.go
@@ -591,9 +591,7 @@ func (fc *funcContext) translateExpr(expr ast.Expr) *expression {
 		case types.MethodVal:
 			return fc.formatExpr(`$methodVal(%s, "%s")`, fc.makeReceiver(e), sel.Obj().(*types.Func).Name())
 		case types.MethodExpr:
-			if !sel.Obj().Exported() {
-				fc.pkgCtx.DeclareDCEDep(sel.Obj())
-			}
+			fc.pkgCtx.DeclareDCEDep(sel.Obj())
 			if _, ok := sel.Recv().Underlying().(*types.Interface); ok {
 				return fc.formatExpr(`$ifaceMethodExpr("%s")`, sel.Obj().(*types.Func).Name())
 			}
@@ -620,10 +618,7 @@ func (fc *funcContext) translateExpr(expr ast.Expr) *expression {
 			if typesutil.IsJsPackage(obj.Pkg()) && obj.Name() == "InternalObject" {
 				return fc.translateExpr(e.Args[0])
 			}
-
-			if inst, has := fc.pkgCtx.Instances[f]; has {
-				fc.pkgCtx.DeclareDCEDepWithInstance(obj, inst)
-			}
+			fc.pkgCtx.DeclareDCEDep(obj)
 			return fc.translateCall(e, sig, fc.translateExpr(f))
 
 		case *ast.SelectorExpr:

diff --git a/compiler/internal/dce/README.md b/compiler/internal/dce/README.md
@@ -17,6 +17,7 @@ The dead-code may be safely eliminated, i.e. not outputted to the JS file(s).
   - [Functions](#functions)
   - [Variables](#variables)
   - [Generics and Instances](#generics-and-instances)
+- [Implementation](#implementation)
 - [Examples](#examples)
   - [Grandmas and Zombies](#grandmas-and-zombies)
   - [Side Effects](#side-effects)
@@ -130,6 +131,35 @@ is `int`. This method in the instance now duck-types to
 `interface { getValues() []int }` and therefore must follow the rules for
 unexported methods.
 
+## Implementation
+
+The implementation uses string matching to indicate aliveness.
+A DCE Info represents a declaration of some code ([`Decl`](../decls.go)).
+The DCE info and dependencies use
+[`types.Object`](https://pkg.go.dev/go/types#Object)'s to determine the
+strings for matching.
+
+When adding dependency to the DCE info, the following string is added:
+
+- The dependency's base string is the path and name (e.g. `foo.Bar`).
+- If the object is a method (with a receiver), a tilde is added
+  (e.g. `foo.Bar~`).
+- If the object is an instance of a generic, the instance types (type arguments)
+  are added (e.g. `foo.Bar[int, string]` or `foo.Bar[int, string]~`).
+  If any of the instance types are declarations then the path and name
+  is used with possible sub-instance type
+  (e.g. `foo.Bar[foo.Foo[map[string]int, bool]]`).
+
+The DCE Info is given an object as the name(s) to use.
+
+- If the object is a method (with a receiver) (e.g. `func (b Bar) baz()`):
+  - The DCE's name is the receiver's DCE's name,
+    i.e. the path and receiver name (e.g. `foo.Bar`).
+  - An additional name is added if the method is unexposed.
+    The additional name is the path and method name with a tilde
+    (e.g. `foo.baz~`)
+- Else the DCE's name is the path and name (e.g. `foo.Bar`).
+
 ## Examples
 
 ### Grandmas and Zombies

diff --git a/compiler/internal/dce/collector.go b/compiler/internal/dce/collector.go
@@ -17,6 +17,7 @@ type Decl interface {
 // Collector is a tool to collect dependencies for a declaration
 // that'll be used in dead-code elimination (DCE).
 type Collector struct {
+	TypeInfo     *types.Info
 	dependencies map[string]struct{}
 }
 
@@ -50,16 +51,6 @@ func (c *Collector) DeclareDCEDepWithInstance(o types.Object, inst types.Instanc
 		return // Dependencies are not being collected.
 	}
 
-	// add object dependency
-	// If the object is a type it will be added as <path>.<name> (e.g. "foo.Bar")
-	// If the object is a method (with a receiver) have a tilde added (e.g. "foo.Bar~")
-	// If the object is an instance of a generic, the type arguments are
-	// added (e.g. "foo.Bar[int, string]" or "foo.Bar[int, string]~").
-	//
-	// The methods are set as dependencies without their receiver type since
-	// the DCE doesn't track if the method is part of an interface or a concrete type.
-	// This means that if a method is used, all types that have this method will be kept
-	// since the method is not tied to a specific type and maybe used via an interface.
 	qualifiedName := o.Pkg().Path() + "." + o.Name()
 	if inst.TypeArgs != nil {
 		tps := make([]string, inst.TypeArgs.Len())
@@ -72,12 +63,4 @@ func (c *Collector) DeclareDCEDepWithInstance(o types.Object, inst types.Instanc
 		qualifiedName += "~"
 	}
 	c.dependencies[qualifiedName] = struct{}{}
-
-	// TODO(gn): Things to test:
-	// - Casting to type (e.g. `type X[T any] interface{ Get() T }; struct Y{}; func (y Y) Get() int { return 42 }; var v X[int] = Y{}`)
-	// - Complex type parameters (e.g. `type X[T any] struct{v T}; func (x X[T]) Get() T { return v }; var v = X[X[X[int]]]{v: X[X[int]]{v: X[int]{v: 42}}}.Get().Get().Get()`)
-	// - Automatic type selection (e.g. Sum[T int | float64](values ...T) T { ** }; var vi = Sum(42, 43), vf = Sum(3.14, 4.25)`)
-	// - Embedded instance in type (e.g. `type X[T any] struct{v T}; type Y[T any] struct{X[T]}; var v = Y[int]{X: X[int]{v: 42}}.X.v`)
-	// - Embedded interface in struct providing the type with an exposed method.
-	// - Type params that are self referencing (e.g. `func Keys[K comparable, V any, M ~map[K]V](m M) { ** }`)
 }
diff --git a/compiler/internal/dce/dce_test.go b/compiler/internal/dce/dce_test.go
@@ -76,15 +76,16 @@ func Test_Collector_Collecting(t *testing.T) {
 func Test_Info_SetNameAndDep(t *testing.T) {
 	tests := []struct {
 		name    string
-		obj     types.Object
+		id      string // identifier for the object
+		src     string // source code to parse and read object from
 		want    Info   // expected Info after SetName
 		wantDep string // expected dep after addDep
 	}{
 		{
 			name: `package`,
-			obj: parseObject(t, `Sarah`,
-				`package jim
-				import Sarah "fmt"`),
+			id:   `Sarah`,
+			src: `package jim
+				import Sarah "fmt"`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Sarah`,
@@ -93,9 +94,9 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed var`,
-			obj: parseObject(t, `Toby`,
-				`package jim
-				var Toby float64`),
+			id:   `Toby`,
+			src: `package jim
+				var Toby float64`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Toby`,
@@ -104,9 +105,9 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed const`,
-			obj: parseObject(t, `Ludo`,
-				`package jim
-				const Ludo int = 42`),
+			id:   `Ludo`,
+			src: `package jim
+				const Ludo int = 42`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Ludo`,
@@ -115,16 +116,16 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `label`,
-			obj: parseObject(t, `Gobo`,
-				`package jim
+			id:   `Gobo`,
+			src: `package jim
 				func main() {
 					i := 0
 				Gobo:
 					i++
 					if i < 10 {
 						goto Gobo
 					}
-				}`),
+				}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Gobo`,
@@ -133,9 +134,9 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed specific type`,
-			obj: parseObject(t, `Jen`,
-				`package jim
-				type Jen struct{}`),
+			id:   `Jen`,
+			src: `package jim
+				type Jen struct{}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Jen`,
@@ -144,9 +145,9 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed generic type`,
-			obj: parseObject(t, `Henson`,
-				`package jim
-				type Henson[T comparable] struct{}`),
+			id:   `Henson`,
+			src: `package jim
+				type Henson[T comparable] struct{}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Henson`,
@@ -155,9 +156,9 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed specific function`,
-			obj: parseObject(t, `Jareth`,
-				`package jim
-				func Jareth() {}`),
+			id:   `Jareth`,
+			src: `package jim
+				func Jareth() {}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Jareth`,
@@ -166,9 +167,9 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed generic function`,
-			obj: parseObject(t, `Didymus`,
-				`package jim
-				func Didymus[T comparable]() {}`),
+			id:   `Didymus`,
+			src: `package jim
+				func Didymus[T comparable]() {}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Didymus`,
@@ -177,10 +178,10 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `exposed specific method`,
-			obj: parseObject(t, `Kira`,
-				`package jim
+			id:   `Kira`,
+			src: `package jim
 				type Fizzgig string
-				func (f Fizzgig) Kira() {}`),
+				func (f Fizzgig) Kira() {}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Fizzgig`,
@@ -189,10 +190,10 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `unexposed specific method`,
-			obj: parseObject(t, `frank`,
-				`package jim
+			id:   `frank`,
+			src: `package jim
 				type Aughra int
-				func (a Aughra) frank() {}`),
+				func (a Aughra) frank() {}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `Aughra`,
@@ -202,16 +203,24 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		},
 		{
 			name: `specific method on unexposed type`,
-			obj: parseObject(t, `Red`,
-				`package jim
+			id:   `Red`,
+			src: `package jim
 				type wembley struct{}
-				func (w wembley) Red() {}`),
+				func (w wembley) Red() {}`,
 			want: Info{
 				importPath:   `jim`,
 				objectFilter: `wembley`,
 			},
 			wantDep: `jim.Red~`,
 		},
+
+		// TODO(gn): Things to test:
+		// - Casting to type (e.g. `type X[T any] interface{ Get() T }; struct Y{}; func (y Y) Get() int { return 42 }; var v X[int] = Y{}`)
+		// - Complex type parameters (e.g. `type X[T any] struct{v T}; func (x X[T]) Get() T { return v }; var v = X[X[X[int]]]{v: X[X[int]]{v: X[int]{v: 42}}}.Get().Get().Get()`)
+		// - Automatic type selection (e.g. Sum[T int | float64](values ...T) T { ** }; var vi = Sum(42, 43), vf = Sum(3.14, 4.25)`)
+		// - Embedded instance in type (e.g. `type X[T any] struct{v T}; type Y[T any] struct{X[T]}; var v = Y[int]{X: X[int]{v: 42}}.X.v`)
+		// - Embedded interface in struct providing the type with an exposed method.
+		// - Type params that are self referencing (e.g. `func Keys[K comparable, V any, M ~map[K]V](m M) { ** }`)
 	}
 
 	t.Run(`SetName`, func(t *testing.T) {
@@ -220,9 +229,11 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 				d := &testDecl{}
 				equal(t, d.Dce().unnamed(), true)
 				equal(t, d.Dce().String(), `[unnamed] . -> []`)
-				t.Log(`object:`, types.ObjectString(tt.obj, nil))
+				info := newTypeInfo()
+				obj := parseObject(t, info, tt.id, tt.src)
+				t.Log(`object:`, types.ObjectString(obj, nil))
 
-				d.Dce().SetName(tt.obj)
+				d.Dce().SetName(obj)
 				equal(t, d.Dce().unnamed(), tt.want.unnamed())
 				equal(t, d.Dce().importPath, tt.want.importPath)
 				equal(t, d.Dce().objectFilter, tt.want.objectFilter)
@@ -236,11 +247,13 @@ func Test_Info_SetNameAndDep(t *testing.T) {
 		for _, tt := range tests {
 			t.Run(tt.name, func(t *testing.T) {
 				d := &testDecl{}
-				t.Log(`object:`, types.ObjectString(tt.obj, nil))
+				info := newTypeInfo()
+				obj := parseObject(t, info, tt.id, tt.src)
+				t.Log(`object:`, types.ObjectString(obj, nil))
 
-				c := Collector{}
+				c := Collector{TypeInfo: info}
 				c.CollectDCEDeps(d, func() {
-					c.DeclareDCEDep(tt.obj)
+					c.DeclareDCEDep(obj)
 				})
 				equal(t, len(d.Dce().deps), 1)
 				equal(t, d.Dce().deps[0], tt.wantDep)
@@ -313,7 +326,7 @@ func Test_Selector_JustVars(t *testing.T) {
 		boromir, gimli, legolas, gandalf,
 	}
 
-	c := Collector{}
+	c := Collector{TypeInfo: newTypeInfo()}
 	c.CollectDCEDeps(frodo, func() {
 		c.DeclareDCEDep(samwise.obj)
 		c.DeclareDCEDep(meri.obj)
@@ -423,7 +436,8 @@ func Test_Selector_JustVars(t *testing.T) {
 }
 
 func Test_Selector_SpecificMethods(t *testing.T) {
-	objects := parseObjects(t,
+	info := newTypeInfo()
+	objects := parseObjects(t, info,
 		`package pratchett
 
 		type rincewind struct{}
@@ -449,7 +463,7 @@ func Test_Selector_SpecificMethods(t *testing.T) {
 	)
 	allDecls := []*testDecl{rincewind, rincewindRun, rincewindHide, vimes, vimesRun, vimesRead, vetinari}
 
-	c := Collector{}
+	c := Collector{TypeInfo: info}
 	c.CollectDCEDeps(rincewindRun, func() {
 		c.DeclareDCEDep(rincewind.obj)
 	})
@@ -541,9 +555,17 @@ func quickVar(pkg *types.Package, name string) *types.Var {
 	return types.NewVar(token.NoPos, pkg, name, types.Typ[types.Int])
 }
 
-func parseObject(t *testing.T, name, source string) types.Object {
+func newTypeInfo() *types.Info {
+	return &types.Info{
+		Defs:      map[*ast.Ident]types.Object{},
+		Uses:      map[*ast.Ident]types.Object{},
+		Instances: map[*ast.Ident]types.Instance{},
+	}
+}
+
+func parseObject(t *testing.T, info *types.Info, name, source string) types.Object {
 	t.Helper()
-	objects := parseObjects(t, source)
+	objects := parseObjects(t, info, source)
 	for _, obj := range objects {
 		if obj.Name() == name {
 			return obj
@@ -553,12 +575,9 @@ func parseObject(t *testing.T, name, source string) types.Object {
 	return nil
 }
 
-func parseObjects(t *testing.T, source string) []types.Object {
+func parseObjects(t *testing.T, info *types.Info, source string) []types.Object {
 	t.Helper()
-	info := &types.Info{
-		Defs: map[*ast.Ident]types.Object{},
-	}
-	parseInfo(t, source, info)
+	parsePackage(t, source, info)
 	objects := make([]types.Object, 0, len(info.Defs))
 	for _, obj := range info.Defs {
 		if obj != nil {
@@ -571,7 +590,7 @@ func parseObjects(t *testing.T, source string) []types.Object {
 	return objects
 }
 
-func parseInfo(t *testing.T, source string, info *types.Info) *types.Package {
+func parsePackage(t *testing.T, source string, info *types.Info) *types.Package {
 	t.Helper()
 	fset := token.NewFileSet()
 	f, err := parser.ParseFile(fset, `test.go`, source, 0)

diff --git a/compiler/internal/dce/info.go b/compiler/internal/dce/info.go
@@ -101,9 +101,4 @@ func (d *Info) setDeps(depSet map[string]struct{}) {
 	}
 	sort.Strings(deps)
 	d.deps = deps
-
-	// TODO(gn): REMOVE
-	//if d.importPath == `main` {
-	//	fmt.Printf(">setDeps: %s\n", d.String())
-	//}
 }