Skip to content

Commit

Permalink
Update for integration tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@grantnelson-wf
grantnelson-wf committed Sep 27, 2024
1 parent 2d1000c commit 6fe8b6e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 117 deletions.
29 changes: 13 additions & 16 deletions compiler/compiler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,7 @@ func TestDeclSelection_RemoveUnusedFuncInstance(t *testing.T) {

sel.DeclCode.IsDead(`^\s*Foo = function`)
sel.DeclCode.IsDead(`^\s*sliceType(\$\d+)? = \$sliceType\(\$Int\)`)

// TODO(gn): This should not be alive because it is not used.
sel.DeclCode.IsAlive(`^\s*Sum\[\d+ /\* int \*/\]`)
sel.DeclCode.IsDead(`^\s*Sum\[\d+ /\* int \*/\]`)
}

func TestDeclSelection_RemoveUnusedStructTypeInstances(t *testing.T) {
Expand All @@ -229,9 +227,8 @@ func TestDeclSelection_RemoveUnusedStructTypeInstances(t *testing.T) {
sel.DeclCode.IsAlive(`^\s*Foo\[\d+ /\* int \*/\] = \$newType`)
sel.DeclCode.IsAlive(`^\s*\$ptrType\(Foo\[\d+ /\* int \*/\]\)\.prototype\.Bar`)

// TODO(gn): This should not be alive because it is not used.
sel.DeclCode.IsAlive(`^\s*Foo\[\d+ /\* float64 \*/\] = \$newType`)
sel.DeclCode.IsAlive(`^\s*\$ptrType\(Foo\[\d+ /\* float64 \*/\]\)\.prototype\.Bar`)
sel.DeclCode.IsDead(`^\s*Foo\[\d+ /\* float64 \*/\] = \$newType`)
sel.DeclCode.IsDead(`^\s*\$ptrType\(Foo\[\d+ /\* float64 \*/\]\)\.prototype\.Bar`)
}

func TestDeclSelection_RemoveUnusedInterfaceTypeInstances(t *testing.T) {
Expand Down Expand Up @@ -262,11 +259,12 @@ func TestDeclSelection_RemoveUnusedInterfaceTypeInstances(t *testing.T) {
sel.InitCode.IsDead(`\$pkg\.F64 = FooBar\[\d+ /\* float64 \*/\]`)

sel.DeclCode.IsAlive(`^\s*FooBar\[\d+ /\* int \*/\]`)
// TODO(gn): Below should be alive because it is an arg to FooBar[int].
// The Foo[int] instance is defined as a parameter in FooBar[int] that is alive.
// However, Foo[int] isn't used directly in the code so it can be removed.
// JS will simply duck-type the Baz object to Foo[int] without Foo[int] specifically defined.
sel.DeclCode.IsDead(`^\s*Foo\[\d+ /\* int \*/\] = \$newType`)

// TODO(gn): Below should be dead because it is only used by a dead init.
sel.DeclCode.IsAlive(`^\s*FooBar\[\d+ /\* float64 \*/\]`)
sel.DeclCode.IsDead(`^\s*FooBar\[\d+ /\* float64 \*/\]`)
sel.DeclCode.IsDead(`^\s*Foo\[\d+ /\* float64 \*/\] = \$newType`)
}

Expand Down Expand Up @@ -295,9 +293,7 @@ func TestDeclSelection_RemoveUnusedMethodWithDifferentSignature(t *testing.T) {

sel.DeclCode.IsAlive(`^\s*Foo = \$newType`)
sel.DeclCode.IsAlive(`\s*\$ptrType\(Foo\)\.prototype\.Bar`)
// TODO(gn): Below should be dead because it is not used even though
// its name matches a used unexported method.
sel.DeclCode.IsAlive(`\s*\$ptrType\(Foo\)\.prototype\.baz`)
sel.DeclCode.IsDead(`\s*\$ptrType\(Foo\)\.prototype\.baz`)

sel.DeclCode.IsAlive(`^\s*Foo2 = \$newType`)
sel.DeclCode.IsAlive(`\s*\$ptrType\(Foo2\)\.prototype\.Bar`)
Expand Down Expand Up @@ -334,10 +330,11 @@ func TestDeclSelection_RemoveUnusedUnexportedMethodInstance(t *testing.T) {

sel.DeclCode.IsAlive(`^\s*Foo\[\d+ /\* uint \*/\] = \$newType`)
sel.DeclCode.IsAlive(`\s*\$ptrType\(Foo\[\d+ /\* uint \*/\]\)\.prototype\.Bar`)
// TODO(gn): All three below should be dead because Foo[uint].baz is unused.
sel.DeclCode.IsAlive(`\s*\$ptrType\(Foo\[\d+ /\* uint \*/\]\)\.prototype\.baz`)
sel.DeclCode.IsAlive(`^\s*Baz\[\d+ /\* uint \*/\] = \$newType`)
sel.DeclCode.IsAlive(`\s*\$ptrType\(Baz\[\d+ /\* uint \*/\]\)\.prototype\.Bar`)

// All three below are dead because Foo[uint].baz is unused.
sel.DeclCode.IsDead(`\s*\$ptrType\(Foo\[\d+ /\* uint \*/\]\)\.prototype\.baz`)
sel.DeclCode.IsDead(`^\s*Baz\[\d+ /\* uint \*/\] = \$newType`)
sel.DeclCode.IsDead(`\s*\$ptrType\(Baz\[\d+ /\* uint \*/\]\)\.prototype\.Bar`)
}

func TestDeclSelection_RemoveUnusedTypeConstraint(t *testing.T) {
Expand Down
40 changes: 28 additions & 12 deletions compiler/internal/dce/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ may be safely eliminated, i.e. not outputted to the JS file(s).

The following is the logic behind the DCE mechanism. Not all of the following
is used since some conditions are difficult to determine even with a lot of
additional information. To ensure that the JS output is fully functional,
we bias the DCE towards things being alive. We'd rather keep something we
don't need than remove something that is needed.
additional information, and because GopherJS stores some additional information
making some parts of DCE unnecessary. To ensure that the JS output is fully
functional, we bias the DCE towards things being alive. We'd rather keep
something we don't need than remove something that is needed.

### Package

Expand Down Expand Up @@ -92,10 +93,14 @@ All the types in the function signatures and embedded interfaces are the
dependents of the interface.

Interfaces may contain exported and unexported function signatures.
If an interface is alive then all of the functions, even the unexported
functions, are alive.
If an interface is alive then all of the functions are alive.
Since there are many ways to wrap a type with an interface, any alive type that
duck-types to an interface must have all of the matching methods alive.
duck-types to an interface must have all of the matching methods also alive.

In theory the unexported functions are also alive however, for GopherJS there
is an exception because duck-typing is handled separately the method
definitions. Those difference are discussed in [Dependencies](#dependencies)
but for this idea we discuss DCE more generally.

Since the exported methods in an alive type will be alive, see
[Named Types](#named-types), the only ones here that need to be considered
Expand Down Expand Up @@ -247,6 +252,13 @@ the unexported method alive. Since the unexported method is only visible in
the package in which it is defined, the package path is included in the
method name.

To simplify the above for GopherJS, we don't look at the receiver for
an unexported method before indicating it is alive. Meaning if there is no
interface, only two named objects with identical unexported methods, the use
of either will indicate a use of both. This will cause slightly more unexported
method to be alive while reducing the complication of type checking which object
or type of object is performing the call.

| Declaration | exported | unexported | non-generic | generic | object name | method name |
|:------------|:--------:|:----------:|:-----------:|:-------:|:------------|:------------|
| variables | █ | █ | █ | n/a | `<package>.<var name>` | |
Expand Down Expand Up @@ -324,18 +336,22 @@ is recursive, e.g. `Foo[Bar[Bar[...]]]`.

### Dependencies

The dependencies are initialized via two paths.

The first is dependencies that are specified in an expression.
The dependencies that are specified in an expression.
For example a function that invokes another function will be dependent on
that invoked function. When a dependency is added it will be added as one
or more names to the declaration that depends on it. It follows the
[naming rules](#naming) so that the dependencies will match correctly.

The second is structural dependencies that are specified automatically while
the declaration is being named. When an interface is named, it will
automatically add all unexported signatures as dependencies via
In theory, structural dependencies would be needed to be added
automatically while the declaration is being named. When an interface is named,
it would automatically add all unexported signatures as dependencies via
`<package path>.<method name>(<parameter type list>)(<result type list>)`.
However, we do not need to do that in GopherJS because we aren't using
the existence of realized methods in duck-typing. GopherJS stores full set
of method information when describing the type so that even when things like
unexported methods in interfaces are removed, duck-typing will still work
correctly. This reduces the size of the code by not keeping a potentially
long method body when the signature is all that is needed.

Currently we don't filter unused packages so there is no need to automatically
add dependencies on the packages themselves. This is also why the package
Expand Down
75 changes: 1 addition & 74 deletions compiler/internal/dce/dce_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,26 +248,6 @@ func Test_Info_SetNameAndDep(t *testing.T) {
objectFilter: `jim.wembley`,
},
},
{
name: `interface with unexported methods setting dependencies`,
obj: parseObject(t, `Hoggle`,
`package jim
type Hoggle interface{
cowardly() bool
loyalTo(goblin string) bool
makePrinceOfTheBogOfEternalStench() error
}`),
want: Info{
objectFilter: `jim.Hoggle`,
// The automatically defined dependencies for unexported methods
// in the interface that match with the methodFilter of unexported methods.
deps: map[string]struct{}{
`jim.cowardly() bool`: {},
`jim.loyalTo(string) bool`: {},
`jim.makePrinceOfTheBogOfEternalStench() error`: {},
},
},
},
{
name: `unexported method resulting in an interface with exported methods`,
obj: parseObject(t, `bear`,
Expand Down Expand Up @@ -439,54 +419,6 @@ func Test_Info_SetNameAndDep(t *testing.T) {
methodFilter: `jim.sidekick() jim.Beaker[any]`,
},
},
{
name: `unexported method in interface from named embedded interface`,
obj: parseObject(t, `Waldorf`,
`package jim
type Statler interface{
boo()
}
type Waldorf interface{
Statler
}`),
want: Info{
objectFilter: `jim.Waldorf`,
deps: map[string]struct{}{
`jim.boo()`: {},
},
},
},
{
name: `unexported method in interface from unnamed embedded interface`,
obj: parseObject(t, `Waldorf`,
`package jim
type Waldorf interface{
interface{
boo()
}
}`),
want: Info{
objectFilter: `jim.Waldorf`,
deps: map[string]struct{}{
`jim.boo()`: {},
},
},
},
{
name: `unexported method on instance of generic interface`,
obj: parseObject(t, `Waldorf`,
`package jim
type Statler[T any] interface{
boo() T
}
type Waldorf Statler[string]`),
want: Info{
objectFilter: `jim.Waldorf`,
deps: map[string]struct{}{
`jim.boo() string`: {},
},
},
},
{
name: `struct with self referencing type parameter constraints`,
obj: parseObject(t, `Keys`,
Expand All @@ -503,7 +435,7 @@ func Test_Info_SetNameAndDep(t *testing.T) {
},
},
{
name: `struct with self referencing type parameter constraints`,
name: `interface with self referencing type parameter constraints`,
obj: parseObject(t, `ElectricMayhem`,
`package jim
type ElectricMayhem[K comparable, V any, M ~map[K]V] interface {
Expand All @@ -513,11 +445,6 @@ func Test_Info_SetNameAndDep(t *testing.T) {
}`),
want: Info{
objectFilter: `jim.ElectricMayhem[comparable, any, ~map[comparable]any]`,
deps: map[string]struct{}{
`jim.keys() []comparable`: {},
`jim.values() []any`: {},
`jim.asMap() ~map[comparable]any`: {},
},
},
},
{
Expand Down
19 changes: 4 additions & 15 deletions compiler/internal/dce/info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ type Info struct {
objectFilter string

// methodFilter is the secondary DCE name for a declaration.
// This will be empty if objectFilter is empty.
// This usually will be empty if objectFilter is empty.
// This will be set to a qualified method name if the objectFilter
// can not determine if the declaration is alive on it's own.
// See ./README.md for more information.
methodFilter string

// List of fully qualified (including package path) DCE symbol identifiers the
// symbol depends on for dead code elimination purposes.
// Set of fully qualified (including package path) DCE symbol
// and/or method names that this DCE declaration depends on.
deps map[string]struct{}
}

Expand All @@ -55,7 +55,7 @@ func (d *Info) String() string {
// unnamed returns true if SetName has not been called for this declaration.
// This indicates that the DCE is not initialized.
func (d *Info) unnamed() bool {
return d.objectFilter == ``
return d.objectFilter == `` && d.methodFilter == ``
}

// isAlive returns true if the declaration is marked as alive.
Expand Down Expand Up @@ -87,17 +87,6 @@ func (d *Info) SetName(o types.Object, tArgs ...types.Type) {

// Determine name(s) for DCE.
d.objectFilter, d.methodFilter = getFilters(o, tArgs)

// Add automatic dependencies for unexported methods on interfaces.
if n, ok := o.Type().(*types.Named); ok {
if it, ok := n.Underlying().(*types.Interface); ok {
for i := it.NumMethods() - 1; i >= 0; i-- {
if m := it.Method(i); !m.Exported() {
d.addDepName(getMethodFilter(m, tArgs))
}
}
}
}
}

// addDep add a declaration dependencies used by DCE
Expand Down

0 comments on commit 6fe8b6e

Please sign in to comment.