From 0bc81aecf47f3a4623dce4189a08c5fb1b04ca71 Mon Sep 17 00:00:00 2001 From: Evgeniy Stratonikov Date: Thu, 30 Sep 2021 11:48:18 +0300 Subject: [PATCH] compiler: do not emit code for unused imported functions Our current algorithm marks function as used if it is called at least ones, even if the callee function is itself unused. This commit implements more clever traversal to collect usage information more precisely. Signed-off-by: Evgeniy Stratonikov --- pkg/compiler/analysis.go | 72 ++++++++++++++++--- pkg/compiler/compiler_test.go | 34 +++++++++ pkg/compiler/function_call_test.go | 27 +++++++ pkg/compiler/testdata/nestedcall/call.go | 30 ++++++++ .../testdata/nestedcall/inner/call.go | 6 ++ pkg/compiler/testdata/notify/event.go | 15 ++++ 6 files changed, 174 insertions(+), 10 deletions(-) create mode 100644 pkg/compiler/testdata/nestedcall/call.go create mode 100644 pkg/compiler/testdata/nestedcall/inner/call.go create mode 100644 pkg/compiler/testdata/notify/event.go diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index 2440ac2dc..cc89fddbf 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -241,34 +241,86 @@ func (c *codegen) fillDocumentInfo() { }) } +// analyzeFuncUsage traverses all code and returns map with functions +// which should be present in the emitted code. +// This is done using BFS starting from exported functions or +// function used in variable declarations (graph edge corresponds to +// function being called in declaration). func (c *codegen) analyzeFuncUsage() funcUsage { - usage := funcUsage{} + type declPair struct { + decl *ast.FuncDecl + importMap map[string]string + path string + } + // nodeCache contains top-level function declarations . + nodeCache := make(map[string]declPair) + diff := funcUsage{} c.ForEachFile(func(f *ast.File, pkg *types.Package) { + var pkgPath string isMain := pkg == c.mainPkg.Pkg + if !isMain { + pkgPath = pkg.Path() + } + ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { case *ast.CallExpr: + // functions invoked in variable declarations in imported packages + // are marked as used. + var name string switch t := n.Fun.(type) { case *ast.Ident: - var pkgPath string - if !isMain { - pkgPath = pkg.Path() - } - usage[c.getIdentName(pkgPath, t.Name)] = true + name = c.getIdentName(pkgPath, t.Name) case *ast.SelectorExpr: - name, _ := c.getFuncNameFromSelector(t) - usage[name] = true + name, _ = c.getFuncNameFromSelector(t) + default: + return true } + diff[name] = true case *ast.FuncDecl: + name := c.getFuncNameFromDecl(pkgPath, n) + // exported functions are always assumed to be used - if isMain && n.Name.IsExported() { - usage[c.getFuncNameFromDecl("", n)] = true + if isMain && n.Name.IsExported() || isInitFunc(n) || isDeployFunc(n) { + diff[name] = true } + nodeCache[name] = declPair{n, c.importMap, pkgPath} + return false // will be processed in the next stage } return true }) }) + + usage := funcUsage{} + for len(diff) != 0 { + nextDiff := funcUsage{} + for name := range diff { + fd, ok := nodeCache[name] + if !ok || usage[name] { + continue + } + usage[name] = true + + old := c.importMap + c.importMap = fd.importMap + ast.Inspect(fd.decl, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.CallExpr: + switch t := n.Fun.(type) { + case *ast.Ident: + nextDiff[c.getIdentName(fd.path, t.Name)] = true + case *ast.SelectorExpr: + name, _ := c.getFuncNameFromSelector(t) + nextDiff[name] = true + } + } + return true + }) + c.importMap = old + } + diff = nextDiff + } return usage } diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index b2f00490e..07ace1e07 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -180,6 +180,40 @@ func TestEventWarnings(t *testing.T) { }) require.NoError(t, err) }) + t.Run("event in imported package", func(t *testing.T) { + t.Run("unused", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/notify" + func Main() int { + return notify.Value + }` + + _, di, err := compiler.CompileWithDebugInfo("eventTest", strings.NewReader(src)) + require.NoError(t, err) + + _, err = compiler.CreateManifest(di, &compiler.Options{NoEventsCheck: true}) + require.NoError(t, err) + }) + t.Run("used", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/notify" + func Main() int { + notify.EmitEvent() + return 42 + }` + + _, di, err := compiler.CompileWithDebugInfo("eventTest", strings.NewReader(src)) + require.NoError(t, err) + + _, err = compiler.CreateManifest(di, &compiler.Options{}) + require.Error(t, err) + + _, err = compiler.CreateManifest(di, &compiler.Options{ + ContractEvents: []manifest.Event{{Name: "Event"}}, + }) + require.NoError(t, err) + }) + }) } func TestNotifyInVerify(t *testing.T) { diff --git a/pkg/compiler/function_call_test.go b/pkg/compiler/function_call_test.go index fb118cffd..a256eb47c 100644 --- a/pkg/compiler/function_call_test.go +++ b/pkg/compiler/function_call_test.go @@ -324,3 +324,30 @@ func TestFunctionUnusedParameters(t *testing.T) { }` eval(t, src, big.NewInt(101)) } + +func TestUnusedFunctions(t *testing.T) { + t.Run("only variable", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/nestedcall" + func Main() int { + return nestedcall.X + }` + + b, err := compiler.Compile("foo", strings.NewReader(src)) + require.NoError(t, err) + require.Equal(t, 3, len(b)) // PUSHINT8 (42) + RET + eval(t, src, big.NewInt(42)) + }) + t.Run("imported function", func(t *testing.T) { + // Check that import map is set correctly during package traversal. + src := `package foo + import inner "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/nestedcall" + func Main() int { + return inner.N() + }` + + _, err := compiler.Compile("foo", strings.NewReader(src)) + require.NoError(t, err) + eval(t, src, big.NewInt(65)) + }) +} diff --git a/pkg/compiler/testdata/nestedcall/call.go b/pkg/compiler/testdata/nestedcall/call.go new file mode 100644 index 000000000..bd05c6f21 --- /dev/null +++ b/pkg/compiler/testdata/nestedcall/call.go @@ -0,0 +1,30 @@ +package nestedcall + +import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/nestedcall/inner" + +// X is what we use. +const X = 42 + +// N returns inner.A(). +func N() int { + return inner.Return65() +} + +// F calls G. +func F() int { + a := 1 + return G() + a +} + +// G calls x and returns y(). +func G() int { + x() + z := 3 + return y() + z +} + +func x() {} +func y() int { + tmp := 10 + return tmp +} diff --git a/pkg/compiler/testdata/nestedcall/inner/call.go b/pkg/compiler/testdata/nestedcall/inner/call.go new file mode 100644 index 000000000..d15dd572a --- /dev/null +++ b/pkg/compiler/testdata/nestedcall/inner/call.go @@ -0,0 +1,6 @@ +package inner + +// Return65 returns 65. +func Return65() int { + return 65 +} diff --git a/pkg/compiler/testdata/notify/event.go b/pkg/compiler/testdata/notify/event.go new file mode 100644 index 000000000..ec1c1b9ae --- /dev/null +++ b/pkg/compiler/testdata/notify/event.go @@ -0,0 +1,15 @@ +package notify + +import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" + +// Value is the constant we use. +const Value = 42 + +// EmitEvent emits some event. +func EmitEvent() { + emitPrivate() +} + +func emitPrivate() { + runtime.Notify("Event") +}