From 1dcbdb011a50cd1acfed5a2483a8c75a475f804c Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 15 Aug 2022 13:15:24 +0300 Subject: [PATCH 1/5] compiler: emit code for unnamed global var decls more careful In case if global var is unnamed (and, as a consequence, unused) and contains a function call inside its value specification, we need to emit code for this var to be able to call the function as it can have side-effects. See the example: ``` package foo import "github.com/nspcc-dev/neo-go/pkg/interop/runtime" var A = f() func Main() int { return 3 } func f() int { runtime.Notify("Valuable notification", 1) return 2 } ``` --- pkg/compiler/analysis.go | 46 +++++++++++++++++++++++++++++----- pkg/compiler/global_test.go | 49 ++++++++++++++++++++++++++++++------- 2 files changed, 80 insertions(+), 15 deletions(-) diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index c447cd5fe..42251053f 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -49,11 +49,15 @@ func (c *codegen) getIdentName(pkg string, name string) string { func (c *codegen) traverseGlobals() bool { var hasDefer bool var n, nConst int + var hasUnusedCall bool var hasDeploy bool c.ForEachFile(func(f *ast.File, pkg *types.Package) { - nv, nc := countGlobals(f) + nv, nc, huc := countGlobals(f, !hasUnusedCall) n += nv nConst += nc + if huc { + hasUnusedCall = true + } if !hasDeploy || !hasDefer { ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { @@ -85,7 +89,10 @@ func (c *codegen) traverseGlobals() bool { lastCnt, maxCnt := -1, -1 c.ForEachPackage(func(pkg *packages.Package) { - if n+nConst > 0 { + // TODO: @optimizeme: it could happen that we don't need the whole set of globals to be emitted. + // We don't need the code for unused var at all, but at the same time we need to emit code for those + // vars that have function call inside. Thus convertGlobals should be able to distinguish these cases. + if n+nConst > 0 || hasUnusedCall { for _, f := range pkg.Syntax { c.fillImportMap(f, pkg) c.convertGlobals(f, pkg.Types) @@ -143,26 +150,37 @@ func (c *codegen) traverseGlobals() bool { // countGlobals counts the global variables in the program to add // them with the stack size of the function. // Second returned argument contains the amount of global constants. -func countGlobals(f ast.Node) (int, int) { +// If checkUnusedCalls set to true then unnamed global variables containing call +// will be searched for and their presence is returned as the last argument. +func countGlobals(f ast.Node, checkUnusedCalls bool) (int, int, bool) { var numVar, numConst int + var hasUnusedCall bool ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { // Skip all function declarations if we have already encountered `defer`. case *ast.FuncDecl: return false // After skipping all funcDecls, we are sure that each value spec - // is a global declared variable or constant. + // is a globally declared variable or constant. case *ast.GenDecl: isVar := n.Tok == token.VAR if isVar || n.Tok == token.CONST { for _, s := range n.Specs { - for _, id := range s.(*ast.ValueSpec).Names { + valueSpec := s.(*ast.ValueSpec) + multiRet := len(valueSpec.Values) != 0 && len(valueSpec.Names) != len(valueSpec.Values) // e.g. var A, B = f() where func f() (int, int) + for j, id := range valueSpec.Names { if id.Name != "_" { if isVar { numVar++ } else { numConst++ } + } else if isVar && len(valueSpec.Values) != 0 && checkUnusedCalls && !hasUnusedCall { + indexToCheck := j + if multiRet { + indexToCheck = 0 + } + hasUnusedCall = containsCall(valueSpec.Values[indexToCheck]) } } } @@ -171,7 +189,23 @@ func countGlobals(f ast.Node) (int, int) { } return true }) - return numVar, numConst + return numVar, numConst, hasUnusedCall +} + +// containsCall traverses node and looks if it contains a function or method call. +func containsCall(n ast.Node) bool { + var hasCall bool + ast.Inspect(n, func(node ast.Node) bool { + switch node.(type) { + case *ast.CallExpr: + hasCall = true + case *ast.Ident: + // Can safely skip idents immediately, we're interested at function calls only. + return false + } + return !hasCall + }) + return hasCall } // isExprNil looks if the given expression is a `nil`. diff --git a/pkg/compiler/global_test.go b/pkg/compiler/global_test.go index 1d910797a..22b1b52ba 100644 --- a/pkg/compiler/global_test.go +++ b/pkg/compiler/global_test.go @@ -13,15 +13,46 @@ import ( ) func TestUnusedGlobal(t *testing.T) { - src := `package foo - const ( - _ int = iota - a - ) - func Main() int { - return 1 - }` - eval(t, src, big.NewInt(1)) + t.Run("simple unused", func(t *testing.T) { + src := `package foo + const ( + _ int = iota + a + ) + func Main() int { + return 1 + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // PUSH1 + RET + }) + t.Run("unused with function call inside", func(t *testing.T) { + t.Run("specification names count matches values count", func(t *testing.T) { + src := `package foo + var control int + var _ = f() + func Main() int { + return control + } + func f() int { + control = 1 + return 5 + }` + eval(t, src, big.NewInt(1)) + }) + t.Run("specification names count differs from values count", func(t *testing.T) { + src := `package foo + var control int + var _, _ = f() + func Main() int { + return control + } + func f() (int, int) { + control = 1 + return 5, 6 + }` + eval(t, src, big.NewInt(1)) + }) + }) } func TestChangeGlobal(t *testing.T) { From 91b36657d6093bd76e4abac8e64468bf62bd458d Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 19 Aug 2022 14:54:42 +0300 Subject: [PATCH 2/5] compiler: do not emit code for unnamed unused variables If variable is unnamed and does not contain function call then it's treated as unused and code generation may be omitted for it initialization/declaration. --- pkg/compiler/codegen.go | 31 ++++++++++++++++----- pkg/compiler/global_test.go | 52 ++++++++++++++++++++++++++++++++++++ pkg/compiler/init_test.go | 2 +- pkg/compiler/inline_test.go | 2 +- pkg/compiler/syscall_test.go | 6 ++--- pkg/compiler/vardecl_test.go | 45 +++++++++++++++++++++++++++++++ pkg/compiler/verify_test.go | 2 +- pkg/compiler/vm_test.go | 29 ++++++++++++++++---- 8 files changed, 151 insertions(+), 18 deletions(-) diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index 0a17a782e..aa5ac2b84 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -596,15 +596,32 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor { } } } - for i := range t.Names { - if len(t.Values) != 0 { - if i == 0 || !multiRet { - ast.Walk(c, t.Values[i]) + for i, id := range t.Names { + if id.Name != "_" { + if len(t.Values) != 0 { + if i == 0 || !multiRet { + ast.Walk(c, t.Values[i]) + } + } else { + c.emitDefault(c.typeOf(t.Type)) } - } else { - c.emitDefault(c.typeOf(t.Type)) + c.emitStoreVar("", t.Names[i].Name) + continue + } + // If var decl contains call then the code should be emitted for it, otherwise - do not evaluate. + if len(t.Values) == 0 { + continue + } + var hasCall bool + if i == 0 || !multiRet { + hasCall = containsCall(t.Values[i]) + } + if hasCall { + ast.Walk(c, t.Values[i]) + } + if hasCall || i != 0 && multiRet { + c.emitStoreVar("", "_") // drop unused after walk } - c.emitStoreVar("", t.Names[i].Name) } } } diff --git a/pkg/compiler/global_test.go b/pkg/compiler/global_test.go index 22b1b52ba..5dbaf50ea 100644 --- a/pkg/compiler/global_test.go +++ b/pkg/compiler/global_test.go @@ -9,6 +9,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/compiler" "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/stretchr/testify/require" ) @@ -52,6 +53,30 @@ func TestUnusedGlobal(t *testing.T) { }` eval(t, src, big.NewInt(1)) }) + t.Run("used", func(t *testing.T) { + src := `package foo + var _, A = f() + func Main() int { + return A + } + func f() (int, int) { + return 5, 6 + }` + eval(t, src, big.NewInt(6)) + }) + }) + t.Run("unused without function call", func(t *testing.T) { + src := `package foo + var _ = 1 + var ( + _ = 2 + 3 + _, _ = 3 + 4, 5 + ) + func Main() int { + return 1 + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // PUSH1 + RET }) } @@ -324,3 +349,30 @@ func TestUnderscoreVarsDontUseSlots(t *testing.T) { src := buf.String() eval(t, src, big.NewInt(count)) } + +func TestUnderscoreGlobalVarDontEmitCode(t *testing.T) { + src := `package foo + var _ int + var _ = 1 + var ( + A = 2 + _ = A + 3 + _, B, _ = 4, 5, 6 + _, C, _ = f(A, B) + ) + var D = 7 // unused but named, so the code is expected + func Main() int { + return 1 + } + func f(a, b int) (int, int, int) { + return 8, 9, 10 + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{4}}, // sslot for A, B, C, D + opcode.PUSH2, opcode.STSFLD0, // store A + opcode.PUSH5, opcode.STSFLD1, // store B + opcode.LDSFLD0, opcode.LDSFLD1, opcode.SWAP, []interface{}{opcode.CALL, []byte{10}}, // evaluate f + opcode.DROP, opcode.STSFLD2, opcode.DROP, // store C + opcode.PUSH7, opcode.STSFLD3, opcode.RET, // store D + opcode.PUSH1, opcode.RET, // Main + []interface{}{opcode.INITSLOT, []byte{0, 2}}, opcode.PUSH10, opcode.PUSH9, opcode.PUSH8, opcode.RET) // f +} diff --git a/pkg/compiler/init_test.go b/pkg/compiler/init_test.go index 45e1ad68c..b2d5d7a99 100644 --- a/pkg/compiler/init_test.go +++ b/pkg/compiler/init_test.go @@ -100,7 +100,7 @@ func TestInitWithNoGlobals(t *testing.T) { func Main() int { return 42 }` - v, s := vmAndCompileInterop(t, src) + v, s, _ := vmAndCompileInterop(t, src) require.NoError(t, v.Run()) assertResult(t, v, big.NewInt(42)) require.True(t, len(s.events) == 1) diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index 9de0379aa..b11cb7911 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -12,7 +12,7 @@ import ( ) func checkCallCount(t *testing.T, src string, expectedCall, expectedInitSlot, expectedLocalsMain int) { - v, sp := vmAndCompileInterop(t, src) + v, sp, _ := vmAndCompileInterop(t, src) mainStart := -1 for _, m := range sp.info.Methods { diff --git a/pkg/compiler/syscall_test.go b/pkg/compiler/syscall_test.go index 9fc86cfdf..22cbb2756 100644 --- a/pkg/compiler/syscall_test.go +++ b/pkg/compiler/syscall_test.go @@ -193,7 +193,7 @@ func TestNotify(t *testing.T) { runtime.Notify("single") }` - v, s := vmAndCompileInterop(t, src) + v, s, _ := vmAndCompileInterop(t, src) v.Estack().PushVal(11) require.NoError(t, v.Run()) @@ -224,7 +224,7 @@ func TestSyscallInGlobalInit(t *testing.T) { func Main() bool { return a }` - v, s := vmAndCompileInterop(t, src) + v, s, _ := vmAndCompileInterop(t, src) s.interops[interopnames.ToID([]byte(interopnames.SystemRuntimeCheckWitness))] = func(v *vm.VM) error { s := v.Estack().Pop().Value().([]byte) require.Equal(t, "5T", string(s)) @@ -479,7 +479,7 @@ func TestInteropTypesComparison(t *testing.T) { b := struct{}{} return a.Equals(b) }` - vm, _ := vmAndCompileInterop(t, src) + vm, _, _ := vmAndCompileInterop(t, src) err := vm.Run() require.Error(t, err) require.True(t, strings.Contains(err.Error(), "invalid conversion: Struct/ByteString"), err) diff --git a/pkg/compiler/vardecl_test.go b/pkg/compiler/vardecl_test.go index 607cd98fa..3759582b3 100644 --- a/pkg/compiler/vardecl_test.go +++ b/pkg/compiler/vardecl_test.go @@ -3,6 +3,8 @@ package compiler_test import ( "math/big" "testing" + + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" ) func TestGenDeclWithMultiRet(t *testing.T) { @@ -29,3 +31,46 @@ func TestGenDeclWithMultiRet(t *testing.T) { eval(t, src, big.NewInt(3)) }) } + +func TestUnderscoreLocalVarDontEmitCode(t *testing.T) { + src := `package foo + type Foo struct { Int int } + func Main() int { + var _ int + var _ = 1 + var ( + A = 2 + _ = A + 3 + _, B, _ = 4, 5, 6 + _, _, _ = f(A, B) // unused, but has function call, so the code is expected + _, C, _ = f(A, B) + ) + var D = 7 // unused but named, so the code is expected + _ = D + var _ = Foo{ Int: 5 } + var fo = Foo{ Int: 3 } + var _ = 1 + A + fo.Int + var _ = fo.GetInt() // unused, but has method call, so the code is expected + return C + } + func f(a, b int) (int, int, int) { + return 8, 9, 10 + } + func (fo Foo) GetInt() int { + return fo.Int + }` + eval(t, src, big.NewInt(9), []interface{}{opcode.INITSLOT, []byte{5, 0}}, // local slot for A, B, C, D, fo + opcode.PUSH2, opcode.STLOC0, // store A + opcode.PUSH5, opcode.STLOC1, // store B + opcode.LDLOC0, opcode.LDLOC1, opcode.SWAP, []interface{}{opcode.CALL, []byte{27}}, // evaluate f() first time + opcode.DROP, opcode.DROP, opcode.DROP, // drop all values from f + opcode.LDLOC0, opcode.LDLOC1, opcode.SWAP, []interface{}{opcode.CALL, []byte{19}}, // evaluate f() second time + opcode.DROP, opcode.STLOC2, opcode.DROP, // store C + opcode.PUSH7, opcode.STLOC3, // store D + opcode.LDLOC3, opcode.DROP, // empty assignment + opcode.PUSH3, opcode.PUSH1, opcode.PACKSTRUCT, opcode.STLOC4, // fo decl + opcode.LDLOC4, []interface{}{opcode.CALL, []byte{12}}, opcode.DROP, // fo.GetInt() + opcode.LDLOC2, opcode.RET, // return C + []interface{}{opcode.INITSLOT, []byte{0, 2}}, opcode.PUSH10, opcode.PUSH9, opcode.PUSH8, opcode.RET, // f + []interface{}{opcode.INITSLOT, []byte{0, 1}}, opcode.LDARG0, opcode.PUSH0, opcode.PICKITEM, opcode.RET) // (fo Foo) GetInt() int +} diff --git a/pkg/compiler/verify_test.go b/pkg/compiler/verify_test.go index 6bb54d4eb..b2dd7834a 100644 --- a/pkg/compiler/verify_test.go +++ b/pkg/compiler/verify_test.go @@ -18,7 +18,7 @@ func TestVerifyGood(t *testing.T) { pub, sig := signMessage(t, msg) src := getVerifyProg(pub, sig) - v, p := vmAndCompileInterop(t, src) + v, p, _ := vmAndCompileInterop(t, src) p.interops[interopnames.ToID([]byte(interopnames.SystemCryptoCheckSig))] = func(v *vm.VM) error { assert.Equal(t, pub, v.Estack().Pop().Bytes()) assert.Equal(t, sig, v.Estack().Pop().Bytes()) diff --git a/pkg/compiler/vm_test.go b/pkg/compiler/vm_test.go index af908da61..867f3aa5a 100644 --- a/pkg/compiler/vm_test.go +++ b/pkg/compiler/vm_test.go @@ -9,9 +9,12 @@ import ( "github.com/nspcc-dev/neo-go/pkg/compiler" "github.com/nspcc-dev/neo-go/pkg/core/interop/interopnames" "github.com/nspcc-dev/neo-go/pkg/core/state" + "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/emit" + "github.com/nspcc-dev/neo-go/pkg/vm/opcode" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -32,9 +35,25 @@ func runTestCases(t *testing.T, tcases []testCase) { } } -func eval(t *testing.T, src string, result interface{}) { - vm, _ := vmAndCompileInterop(t, src) +func eval(t *testing.T, src string, result interface{}, expectedOps ...interface{}) []byte { + vm, _, script := vmAndCompileInterop(t, src) + if len(expectedOps) != 0 { + expected := io.NewBufBinWriter() + for _, op := range expectedOps { + switch typ := op.(type) { + case opcode.Opcode: + emit.Opcodes(expected.BinWriter, typ) + case []interface{}: + emit.Instruction(expected.BinWriter, typ[0].(opcode.Opcode), typ[1].([]byte)) + default: + t.Fatalf("unexpected evaluation operation: %v", typ) + } + } + + require.Equal(t, expected.Bytes(), script) + } runAndCheck(t, vm, result) + return script } func runAndCheck(t *testing.T, v *vm.VM, result interface{}) { @@ -61,11 +80,11 @@ func assertResult(t *testing.T, vm *vm.VM, result interface{}) { } func vmAndCompile(t *testing.T, src string) *vm.VM { - v, _ := vmAndCompileInterop(t, src) + v, _, _ := vmAndCompileInterop(t, src) return v } -func vmAndCompileInterop(t *testing.T, src string) (*vm.VM, *storagePlugin) { +func vmAndCompileInterop(t *testing.T, src string) (*vm.VM, *storagePlugin, []byte) { vm := vm.New() storePlugin := newStoragePlugin() @@ -77,7 +96,7 @@ func vmAndCompileInterop(t *testing.T, src string) (*vm.VM, *storagePlugin) { storePlugin.info = di invokeMethod(t, testMainIdent, b.Script, vm, di) - return vm, storePlugin + return vm, storePlugin, b.Script } func invokeMethod(t *testing.T, method string, script []byte, v *vm.VM, di *compiler.DebugInfo) { From 1e6b70d570eaa42c551d6096fe9bff6f2d89ef23 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 25 Aug 2022 12:06:19 +0300 Subject: [PATCH 3/5] compiler: adjust TestInline template Move all auxiliary function declaration after Main, so that INITSLOT instructions counter works properly. `vmAndCompileInterop` loads program and moves nextIP to the Main function offset if there's no _init function. If _init is there, then nextIP will be moved to the start of _init. In TestInline we don't handle instructions properly (CALL/JMP don't change nextIP), we just perform instruction traversal from the start point via Next(), thus INITSLOT counter value depends on the starting instruction, which depends on _init presence. --- pkg/compiler/inline_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index b11cb7911..fb4125543 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -55,13 +55,13 @@ func TestInline(t *testing.T) { a int b pair } - // local alias - func sum(a, b int) int { - return 42 - } var Num = 1 func Main() int { %s + } + // local alias + func sum(a, b int) int { + return 42 }` t.Run("no return", func(t *testing.T) { src := fmt.Sprintf(srcTmpl, `inline.NoArgsNoReturn() From 800321db06489c94e9d0b034703e97cf2217778c Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Thu, 4 Aug 2022 17:47:32 +0300 Subject: [PATCH 4/5] compiler: rename named unused global vars to "_" So that (*codegen).Visit is able to omit code generation for these unused global vars. The most tricky part is to detect unused global variables, it is done in several steps: 1. Collect the set of named used/unused global vars. 2. Collect the set of globally declared expressions that contain function calls. 3. Pick up global vars from the set made at step 2. 4. Traverse used functions and puck up those global vars that are used from these functions. 5. Rename all globals that are presented in the set made at step 1 but are not presented in the set made on step 3 or step 4. --- pkg/compiler/analysis.go | 241 +++++++- pkg/compiler/codegen.go | 4 +- pkg/compiler/global_test.go | 556 +++++++++++++++++- pkg/compiler/inline_test.go | 23 +- pkg/compiler/testdata/foo/foo.go | 2 +- .../testdata/globalvar/funccall/main.go | 18 + pkg/compiler/testdata/globalvar/main.go | 14 + .../testdata/globalvar/nested1/main.go | 29 + .../testdata/globalvar/nested2/main.go | 20 + .../testdata/globalvar/nested3/main.go | 18 + 10 files changed, 903 insertions(+), 22 deletions(-) create mode 100644 pkg/compiler/testdata/globalvar/funccall/main.go create mode 100644 pkg/compiler/testdata/globalvar/main.go create mode 100644 pkg/compiler/testdata/globalvar/nested1/main.go create mode 100644 pkg/compiler/testdata/globalvar/nested2/main.go create mode 100644 pkg/compiler/testdata/globalvar/nested3/main.go diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index 42251053f..45fca37ab 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -89,13 +89,10 @@ func (c *codegen) traverseGlobals() bool { lastCnt, maxCnt := -1, -1 c.ForEachPackage(func(pkg *packages.Package) { - // TODO: @optimizeme: it could happen that we don't need the whole set of globals to be emitted. - // We don't need the code for unused var at all, but at the same time we need to emit code for those - // vars that have function call inside. Thus convertGlobals should be able to distinguish these cases. if n+nConst > 0 || hasUnusedCall { for _, f := range pkg.Syntax { c.fillImportMap(f, pkg) - c.convertGlobals(f, pkg.Types) + c.convertGlobals(f) } } for _, f := range pkg.Syntax { @@ -169,7 +166,7 @@ func countGlobals(f ast.Node, checkUnusedCalls bool) (int, int, bool) { valueSpec := s.(*ast.ValueSpec) multiRet := len(valueSpec.Values) != 0 && len(valueSpec.Names) != len(valueSpec.Values) // e.g. var A, B = f() where func f() (int, int) for j, id := range valueSpec.Names { - if id.Name != "_" { + if id.Name != "_" { // If variable has name, then it's treated as used - that's countGlobals' caller responsibility to guarantee that. if isVar { numVar++ } else { @@ -284,21 +281,45 @@ func (c *codegen) fillDocumentInfo() { }) } -// analyzeFuncUsage traverses all code and returns a map with functions +// analyzeFuncAndGlobalVarUsage traverses all code and returns a map with functions // which should be present in the emitted code. // This is done using BFS starting from exported functions or // the function used in variable declarations (graph edge corresponds to -// the function being called in declaration). -func (c *codegen) analyzeFuncUsage() funcUsage { +// the function being called in declaration). It also analyzes global variables +// usage preserving the same traversal strategy and rules. Unused global variables +// are renamed to "_" in the end. Global variable is treated as "used" iff: +// 1. It belongs either to main or to exported package AND is used directly from the exported (or _init\_deploy) method of the main package. +// 2. It belongs either to main or to exported package AND is used non-directly from the exported (or _init\_deploy) method of the main package +// (e.g. via series of function calls or in some expression that is "used"). +// 3. It belongs either to main or to exported package AND contains function call inside its value definition. +func (c *codegen) analyzeFuncAndGlobalVarUsage() funcUsage { type declPair struct { decl *ast.FuncDecl importMap map[string]string path string } - - // nodeCache contains top-level function declarations . + // globalVar represents a global variable declaration node with the corresponding package context. + type globalVar struct { + decl *ast.GenDecl // decl contains global variables declaration node (there can be multiple declarations in a single node). + specIdx int // specIdx is the index of variable specification in the list of GenDecl specifications. + varIdx int // varIdx is the index of variable name in the specification names. + ident *ast.Ident // ident is a named global variable identifier got from the specified node. + importMap map[string]string + path string + } + // nodeCache contains top-level function declarations. nodeCache := make(map[string]declPair) + // globalVarsCache contains both used and unused declared named global vars. + globalVarsCache := make(map[string]globalVar) + // diff contains used functions that are not yet marked as "used" and those definition + // requires traversal in the subsequent stages. diff := funcUsage{} + // globalVarsDiff contains used named global variables that are not yet marked as "used" + // and those declaration requires traversal in the subsequent stages. + globalVarsDiff := funcUsage{} + // usedExpressions contains a set of ast.Nodes that are used in the program and need to be evaluated + // (either they are used from the used functions OR belong to global variable declaration and surrounded by a function call) + var usedExpressions []nodeContext c.ForEachFile(func(f *ast.File, pkg *types.Package) { var pkgPath string isMain := pkg == c.mainPkg.Types @@ -350,6 +371,44 @@ func (c *codegen) analyzeFuncUsage() funcUsage { } nodeCache[name] = declPair{n, c.importMap, pkgPath} return false // will be processed in the next stage + case *ast.GenDecl: + // After skipping all funcDecls, we are sure that each value spec + // is a globally declared variable or constant. We need to gather global + // vars from both main and imported packages. + if n.Tok == token.VAR { + for i, s := range n.Specs { + valSpec := s.(*ast.ValueSpec) + for j, id := range valSpec.Names { + if id.Name != "_" { + name := c.getIdentName(pkgPath, id.Name) + globalVarsCache[name] = globalVar{ + decl: n, + specIdx: i, + varIdx: j, + ident: id, + importMap: c.importMap, + path: pkgPath, + } + } + // Traverse both named/unnamed global variables, check whether function/method call + // is present inside variable value and if so, mark all its children as "used" for + // further traversal and evaluation. + if len(valSpec.Values) == 0 { + continue + } + multiRet := len(valSpec.Values) != len(valSpec.Names) + if (j == 0 || !multiRet) && containsCall(valSpec.Values[j]) { + usedExpressions = append(usedExpressions, nodeContext{ + node: valSpec.Values[j], + path: pkgPath, + importMap: c.importMap, + typeInfo: c.typeInfo, + currPkg: c.currPkg, + }) + } + } + } + } } return true }) @@ -358,9 +417,24 @@ func (c *codegen) analyzeFuncUsage() funcUsage { return nil } + // Handle nodes that contain (or surrounded by) function calls and are a part + // of global variable declaration. + c.pickVarsFromNodes(usedExpressions, func(name string) { + if _, gOK := globalVarsCache[name]; gOK { + globalVarsDiff[name] = true + } + }) + + // Traverse the set of upper-layered used functions and construct the functions' usage map. + // At the same time, go through the whole set of used functions and mark global vars used + // from these functions as "used". Also mark the global variables from the previous step + // and their children as "used". usage := funcUsage{} - for len(diff) != 0 { + globalVarsUsage := funcUsage{} + for len(diff) != 0 || len(globalVarsDiff) != 0 { nextDiff := funcUsage{} + nextGlobalVarsDiff := funcUsage{} + usedExpressions = usedExpressions[:0] for name := range diff { fd, ok := nodeCache[name] if !ok || usage[name] { @@ -388,12 +462,157 @@ func (c *codegen) analyzeFuncUsage() funcUsage { } return true }) + usedExpressions = append(usedExpressions, nodeContext{ + node: fd.decl.Body, + path: fd.path, + importMap: c.importMap, + typeInfo: c.typeInfo, + currPkg: c.currPkg, + }) } + + // Traverse used global vars in a separate cycle so that we're sure there's no other unrelated vars. + // Mark their children as "used". + for name := range globalVarsDiff { + fd, ok := globalVarsCache[name] + if !ok || globalVarsUsage[name] { + continue + } + globalVarsUsage[name] = true + pkg := c.mainPkg + if fd.path != "" { + pkg = c.packageCache[fd.path] + } + valSpec := fd.decl.Specs[fd.specIdx].(*ast.ValueSpec) + if len(valSpec.Values) == 0 { + continue + } + multiRet := len(valSpec.Values) != len(valSpec.Names) + if fd.varIdx == 0 || !multiRet { + usedExpressions = append(usedExpressions, nodeContext{ + node: valSpec.Values[fd.varIdx], + path: fd.path, + importMap: fd.importMap, + typeInfo: pkg.TypesInfo, + currPkg: pkg, + }) + } + } + c.pickVarsFromNodes(usedExpressions, func(name string) { + if _, gOK := globalVarsCache[name]; gOK { + nextGlobalVarsDiff[name] = true + } + }) diff = nextDiff + globalVarsDiff = nextGlobalVarsDiff + } + + // Tiny hack: rename all remaining unused global vars. After that these unused + // vars will be handled as any other unnamed unused variables, i.e. + // c.traverseGlobals() won't take them into account during static slot creation + // and the code won't be emitted for them. + for name, node := range globalVarsCache { + if _, ok := globalVarsUsage[name]; !ok { + node.ident.Name = "_" + } } return usage } +// nodeContext contains ast node with the corresponding import map, type info and package information +// required to retrieve fully qualified node name (if so). +type nodeContext struct { + node ast.Node + path string + importMap map[string]string + typeInfo *types.Info + currPkg *packages.Package +} + +// derive returns provided node with the parent's context. +func (c nodeContext) derive(n ast.Node) nodeContext { + return nodeContext{ + node: n, + path: c.path, + importMap: c.importMap, + typeInfo: c.typeInfo, + currPkg: c.currPkg, + } +} + +// pickVarsFromNodes searches for variables used in the given set of nodes +// calling markAsUsed for each variable. Be careful while using codegen after +// pickVarsFromNodes, it changes importMap, currPkg and typeInfo. +func (c *codegen) pickVarsFromNodes(nodes []nodeContext, markAsUsed func(name string)) { + for len(nodes) != 0 { + var nextExprToCheck []nodeContext + for _, val := range nodes { + // Set variable context for proper name extraction. + c.importMap = val.importMap + c.currPkg = val.currPkg + c.typeInfo = val.typeInfo + ast.Inspect(val.node, func(node ast.Node) bool { + switch n := node.(type) { + case *ast.KeyValueExpr: // var _ = f() + CustomInt{Int: Unused}.Int + 3 => mark Unused as "used". + nextExprToCheck = append(nextExprToCheck, val.derive(n.Value)) + return false + case *ast.CallExpr: + switch t := n.Fun.(type) { + case *ast.Ident: + // Do nothing, used functions are handled in a separate cycle. + case *ast.SelectorExpr: + nextExprToCheck = append(nextExprToCheck, val.derive(t)) + } + for _, arg := range n.Args { + switch arg.(type) { + case *ast.BasicLit: + default: + nextExprToCheck = append(nextExprToCheck, val.derive(arg)) + } + } + return false + case *ast.SelectorExpr: + if c.typeInfo.Selections[n] != nil { + switch t := n.X.(type) { + case *ast.Ident: + nextExprToCheck = append(nextExprToCheck, val.derive(t)) + case *ast.CompositeLit: + nextExprToCheck = append(nextExprToCheck, val.derive(t)) + case *ast.SelectorExpr: // imp_pkg.Anna.GetAge() => mark Anna (exported global struct) as used. + nextExprToCheck = append(nextExprToCheck, val.derive(t)) + } + } else { + ident := n.X.(*ast.Ident) + name := c.getIdentName(ident.Name, n.Sel.Name) + markAsUsed(name) + } + return false + case *ast.CompositeLit: // var _ = f(1) + []int{1, Unused, 3}[1] => mark Unused as "used". + for _, e := range n.Elts { + switch e.(type) { + case *ast.BasicLit: + default: + nextExprToCheck = append(nextExprToCheck, val.derive(e)) + } + } + return false + case *ast.Ident: + name := c.getIdentName(val.path, n.Name) + markAsUsed(name) + return false + case *ast.DeferStmt: + nextExprToCheck = append(nextExprToCheck, val.derive(n.Call.Fun)) + return false + case *ast.BasicLit: + return false + } + return true + }) + } + nodes = nextExprToCheck + } +} + func isGoBuiltin(name string) bool { for i := range goBuiltins { if name == goBuiltins[i] { diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go index aa5ac2b84..d74a69700 100644 --- a/pkg/compiler/codegen.go +++ b/pkg/compiler/codegen.go @@ -350,7 +350,7 @@ func (c *codegen) emitDefault(t types.Type) { // convertGlobals traverses the AST and only converts global declarations. // If we call this in convertFuncDecl, it will load all global variables // into the scope of the function. -func (c *codegen) convertGlobals(f *ast.File, _ *types.Package) { +func (c *codegen) convertGlobals(f *ast.File) { ast.Inspect(f, func(node ast.Node) bool { switch n := node.(type) { case *ast.FuncDecl: @@ -2136,7 +2136,7 @@ func (c *codegen) compile(info *buildInfo, pkg *packages.Package) error { c.mainPkg = pkg c.analyzePkgOrder() c.fillDocumentInfo() - funUsage := c.analyzeFuncUsage() + funUsage := c.analyzeFuncAndGlobalVarUsage() if c.prog.Err != nil { return c.prog.Err } diff --git a/pkg/compiler/global_test.go b/pkg/compiler/global_test.go index 5dbaf50ea..46ff61535 100644 --- a/pkg/compiler/global_test.go +++ b/pkg/compiler/global_test.go @@ -63,6 +63,7 @@ func TestUnusedGlobal(t *testing.T) { return 5, 6 }` eval(t, src, big.NewInt(6)) + checkInstrCount(t, src, 1, 1, 0, 0) // sslot for A, single call to f }) }) t.Run("unused without function call", func(t *testing.T) { @@ -80,6 +81,552 @@ func TestUnusedGlobal(t *testing.T) { }) } +func TestUnusedOptimizedGlobalVar(t *testing.T) { + t.Run("unused, no initialization", func(t *testing.T) { + src := `package foo + var A int + var ( + B int + C, D, E int + ) + func Main() int { + return 1 + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // Main + }) + t.Run("used, no initialization", func(t *testing.T) { + src := `package foo + var A int + func Main() int { + return A + }` + eval(t, src, big.NewInt(0)) + checkInstrCount(t, src, 1, 0, 0, 0) // sslot for A + }) + t.Run("used by unused var, no initialization", func(t *testing.T) { + src := `package foo + var Unused int + var Unused2 = Unused + 1 + func Main() int { + return 1 + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // Main + }) + t.Run("unused, with initialization", func(t *testing.T) { + src := `package foo + var Unused = 1 + func Main() int { + return 2 + }` + prog := eval(t, src, big.NewInt(2)) + require.Equal(t, 2, len(prog)) // Main + }) + t.Run("unused, with initialization by used var", func(t *testing.T) { + src := `package foo + var ( + A = 1 + B, Unused, C = f(), A + 2, 3 // the code for Unused initialization won't be emitted as it's a pure expression without function calls + Unused2 = 4 + ) + var Unused3 = 5 + func Main() int { + return A + C + } + func f() int { + return 4 + }` + eval(t, src, big.NewInt(4), []interface{}{opcode.INITSSLOT, []byte{2}}, // sslot for A and C + opcode.PUSH1, opcode.STSFLD0, // store A + []interface{}{opcode.CALL, []byte{10}}, opcode.DROP, // evaluate B and drop + opcode.PUSH3, opcode.STSFLD1, opcode.RET, // store C + opcode.LDSFLD0, opcode.LDSFLD1, opcode.ADD, opcode.RET, // Main + opcode.PUSH4, opcode.RET) // f + }) + t.Run("used by unused var, with initialization", func(t *testing.T) { + src := `package foo + var ( + Unused1 = 1 + Unused2 = Unused1 + 1 + ) + func Main() int { + return 1 + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // Main + }) + t.Run("used with combination of nested unused", func(t *testing.T) { + src := `package foo + var ( + A = 1 + Unused1 = 2 + Unused2 = Unused1 + 1 + ) + func Main() int { + return A + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for A + opcode.PUSH1, opcode.STSFLD0, opcode.RET, // store A + opcode.LDSFLD0, opcode.RET) // Main + }) + t.Run("single var stmt with both used and unused vars", func(t *testing.T) { + src := `package foo + var A, Unused1, B, Unused2 = 1, 2, 3, 4 + func Main() int { + return A + B + }` + eval(t, src, big.NewInt(4), []interface{}{opcode.INITSSLOT, []byte{2}}, // sslot for A and B + opcode.PUSH1, opcode.STSFLD0, // store A + opcode.PUSH3, opcode.STSFLD1, opcode.RET, // store B + opcode.LDSFLD0, opcode.LDSFLD1, opcode.ADD, opcode.RET) // Main + }) + t.Run("single var decl token with multiple var specifications", func(t *testing.T) { + src := `package foo + var ( + A, Unused1, B, Unused2 = 1, 2, 3, 4 + C, Unused3 int + ) + func Main() int { + return A + B + C + }` + eval(t, src, big.NewInt(4), []interface{}{opcode.INITSSLOT, []byte{3}}, // sslot for A, B, C + opcode.PUSH1, opcode.STSFLD0, // store A + opcode.PUSH3, opcode.STSFLD1, // store B + opcode.PUSH0, opcode.STSFLD2, opcode.RET, // store C + opcode.LDSFLD0, opcode.LDSFLD1, opcode.ADD, opcode.LDSFLD2, opcode.ADD, opcode.RET) // Main + }) + t.Run("function as unused var value", func(t *testing.T) { + src := `package foo + var A, Unused1 = 1, f() + func Main() int { + return A + } + func f() int { + return 2 + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for A + opcode.PUSH1, opcode.STSFLD0, // store A + []interface{}{opcode.CALL, []byte{6}}, opcode.DROP, opcode.RET, // evaluate Unused1 (call to f) and drop its value + opcode.LDSFLD0, opcode.RET, // Main + opcode.PUSH2, opcode.RET) // f + }) + t.Run("function as unused struct field", func(t *testing.T) { + src := `package foo + type Str struct { Int int } + var _ = Str{Int: f()} + func Main() int { + return 1 + } + func f() int { + return 2 + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.CALL, []byte{8}}, opcode.PUSH1, opcode.PACKSTRUCT, opcode.DROP, opcode.RET, // evaluate struct val + opcode.PUSH1, opcode.RET, // Main + opcode.PUSH2, opcode.RET) // f + }) + t.Run("used in unused function", func(t *testing.T) { + src := `package foo + var Unused1, Unused2, Unused3 = 1, 2, 3 + func Main() int { + return 1 + } + func unused1() int { + return Unused1 + } + func unused2() int { + return Unused1 + unused1() + } + func unused3() int { + return Unused2 + unused2() + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // Main + }) + t.Run("used in used function", func(t *testing.T) { + src := `package foo + var A = 1 + func Main() int { + return f() + } + func f() int { + return A + }` + eval(t, src, big.NewInt(1)) + checkInstrCount(t, src, 1, 1, 0, 0) + }) + t.Run("unused, initialized via init", func(t *testing.T) { + src := `package foo + var A int + func Main() int { + return 2 + } + func init() { + A = 1 // Although A is unused from exported functions, it's used from init(), so it should be mark as "used" and stored. + }` + eval(t, src, big.NewInt(2)) + checkInstrCount(t, src, 1, 0, 0, 0) + }) + t.Run("used, initialized via init", func(t *testing.T) { + src := `package foo + var A int + func Main() int { + return A + } + func init() { + A = 1 + }` + eval(t, src, big.NewInt(1)) + checkInstrCount(t, src, 1, 0, 0, 0) + }) + t.Run("unused, initialized by function call", func(t *testing.T) { + t.Run("unnamed", func(t *testing.T) { + src := `package foo + var _ = f() + func Main() int { + return 1 + } + func f() int { + return 2 + }` + eval(t, src, big.NewInt(1)) + checkInstrCount(t, src, 0, 1, 0, 0) + }) + t.Run("named", func(t *testing.T) { + src := `package foo + var A = f() + func Main() int { + return 1 + } + func f() int { + return 2 + }` + eval(t, src, big.NewInt(1)) + checkInstrCount(t, src, 0, 1, 0, 0) + }) + t.Run("named, with dependency on unused var", func(t *testing.T) { + src := `package foo + var ( + A = 1 + B = A + 1 // To check nested ident values. + C = 3 + D = B + f() + C // To check that both idents (before and after the call to f) will be marked as "used". + E = C + 1 // Unused, no code expected. + ) + func Main() int { + return 1 + } + func f() int { + return 2 + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{3}}, // sslot for A + opcode.PUSH1, opcode.STSFLD0, // store A + opcode.LDSFLD0, opcode.PUSH1, opcode.ADD, opcode.STSFLD1, // store B + opcode.PUSH3, opcode.STSFLD2, // store C + opcode.LDSFLD1, []interface{}{opcode.CALL, []byte{9}}, opcode.ADD, opcode.LDSFLD2, opcode.ADD, opcode.DROP, opcode.RET, // evaluate D and drop + opcode.PUSH1, opcode.RET, // Main + opcode.PUSH2, opcode.RET) // f + }) + t.Run("named, with dependency on unused var ident inside function call", func(t *testing.T) { + src := `package foo + var A = 1 + var B = f(A) + func Main() int { + return 1 + } + func f(a int) int { + return a + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for A + opcode.PUSH1, opcode.STSFLD0, // store A + opcode.LDSFLD0, []interface{}{opcode.CALL, []byte{6}}, opcode.DROP, opcode.RET, // evaluate B and drop + opcode.PUSH1, opcode.RET, // Main + []interface{}{opcode.INITSLOT, []byte{0, 1}}, opcode.LDARG0, opcode.RET) // f + }) + t.Run("named, inside multi-specs and multi-vals var declaration", func(t *testing.T) { + src := `package foo + var ( + Unused = 1 + Unused1, A, Unused2 = 2, 3 + f(), 4 + ) + func Main() int { + return 1 + } + func f() int { + return 5 + }` + eval(t, src, big.NewInt(1), opcode.PUSH3, []interface{}{opcode.CALL, []byte{7}}, opcode.ADD, opcode.DROP, opcode.RET, // evaluate and drop A + opcode.PUSH1, opcode.RET, // Main + opcode.PUSH5, opcode.RET) // f + }) + t.Run("unnamed + unused", func(t *testing.T) { + src := `package foo + var A = 1 // At least one global variable is used, thus, the whole set of package variables will be walked. + var B = 2 + var _ = B + 1 // This variable is unnamed and doesn't contain call, thus its children won't be marked as "used". + func Main() int { + return A + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for A + opcode.PUSH1, opcode.STSFLD0, opcode.RET, // store A + opcode.LDSFLD0, opcode.RET) // Main + }) + t.Run("mixed value", func(t *testing.T) { + src := `package foo + var control int // At least one global variable is used, thus the whole set of package variables will be walked. + var B = 2 + var _ = 1 + f() + B // This variable is unnamed but contains call, thus its children will be marked as "used". + func Main() int { + return control + } + func f() int { + control = 1 + return 3 + }` + eval(t, src, big.NewInt(1)) + checkInstrCount(t, src, 2 /* control + B */, 1, 0, 0) + }) + t.Run("multiple function return values", func(t *testing.T) { + src := `package foo + var A, B = f() + func Main() int { + return A + } + func f() (int, int) { + return 3, 4 + }` + eval(t, src, big.NewInt(3), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for A + []interface{}{opcode.CALL, []byte{7}}, opcode.STSFLD0, opcode.DROP, opcode.RET, // evaluate and store A, drop B + opcode.LDSFLD0, opcode.RET, // Main + opcode.PUSH4, opcode.PUSH3, opcode.RET) // f + }) + t.Run("constant in declaration", func(t *testing.T) { + src := `package foo + const A = 5 + var Unused = 1 + A + func Main() int { + return 1 + }` + prog := eval(t, src, big.NewInt(1)) + require.Equal(t, 2, len(prog)) // Main + }) + t.Run("mixed expression", func(t *testing.T) { + src := `package foo + type CustomInt struct { + Int int + } + var A = CustomInt{Int: 2} + var B = f(3) + A.f(1) + func Main() int { + return 1 + } + func f(a int) int { + return a + } + func (i CustomInt) f(a int) int { // has the same name as f + return i.Int + a + }` + eval(t, src, big.NewInt(1)) + checkInstrCount(t, src, 1 /* A */, 2, 2, 0) + }) + }) + t.Run("mixed nested expressions", func(t *testing.T) { + src := `package foo + type CustomInt struct { Int int} // has the same field name as Int variable, important for test + var A = CustomInt{Int: 2} + var B = f(A.Int) + var Unused = 4 + var Int = 5 // unused and MUST NOT be treated as "used" + var C = CustomInt{Int: Unused}.Int + f(1) // uses Unused => Unused should be marked as "used" + func Main() int { + return 1 + } + func f(a int) int { + return a + } + func (i CustomInt) f(a int) int { // has the same name as f + return i.Int + a + }` + eval(t, src, big.NewInt(1)) + }) + t.Run("composite literal", func(t *testing.T) { + src := `package foo + var A = 2 + var B = []int{1, A, 3}[1] + var C = f(1) + B + func Main() int { + return 1 + } + func f(a int) int { + return a + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{2}}, // sslot for A, B + opcode.PUSH2, opcode.STSFLD0, // store A + opcode.PUSH3, opcode.LDSFLD0, opcode.PUSH1, opcode.PUSH3, opcode.PACK, opcode.PUSH1, opcode.PICKITEM, opcode.STSFLD1, // evaluate B + opcode.PUSH1, []interface{}{opcode.CALL, []byte{8}}, opcode.LDSFLD1, opcode.ADD, opcode.DROP, opcode.RET, // evalute C and drop + opcode.PUSH1, opcode.RET, // Main + []interface{}{opcode.INITSLOT, []byte{0, 1}}, opcode.LDARG0, opcode.RET) // f + }) + t.Run("index expression", func(t *testing.T) { + src := `package foo + var Unused = 2 + var A = f(1) + []int{1, 2, 3}[Unused] // index expression + func Main() int { + return 1 + } + func f(a int) int { + return a + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for Unused + opcode.PUSH2, opcode.STSFLD0, // store Unused + opcode.PUSH1, []interface{}{opcode.CALL, []byte{14}}, // call f(1) + opcode.PUSH3, opcode.PUSH2, opcode.PUSH1, opcode.PUSH3, opcode.PACK, opcode.LDSFLD0, opcode.PICKITEM, // eval index expression + opcode.ADD, opcode.DROP, opcode.RET, // eval and drop A + opcode.PUSH1, opcode.RET, // Main + []interface{}{opcode.INITSLOT, []byte{0, 1}}, opcode.LDARG0, opcode.RET) // f(a) + }) + t.Run("used via nested function calls", func(t *testing.T) { + src := `package foo + var A = 1 + func Main() int { + return f() + } + func f() int { + return g() + } + func g() int { + return A + }` + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for A + opcode.PUSH1, opcode.STSFLD0, opcode.RET, // store A + []interface{}{opcode.CALL, []byte{3}}, opcode.RET, // Main + []interface{}{opcode.CALL, []byte{3}}, opcode.RET, // f + opcode.LDSFLD0, opcode.RET) // g + }) + t.Run("struct field name matches global var name", func(t *testing.T) { + src := `package foo + type CustomStr struct { Int int } + var str = CustomStr{Int: 2} + var Int = 5 // Unused and the code must not be emitted. + func Main() int { + return str.Int + }` + eval(t, src, big.NewInt(2), []interface{}{opcode.INITSSLOT, []byte{1}}, // sslot for str + opcode.PUSH2, opcode.PUSH1, opcode.PACKSTRUCT, opcode.STSFLD0, opcode.RET, // store str + opcode.LDSFLD0, opcode.PUSH0, opcode.PICKITEM, opcode.RET) // Main + }) + t.Run("var as a struct field initializer", func(t *testing.T) { + src := `package foo + type CustomStr struct { Int int } + var A = 5 + var Int = 6 // Unused + func Main() int { + return CustomStr{Int: A}.Int + }` + eval(t, src, big.NewInt(5)) + }) + t.Run("argument of globally called function", func(t *testing.T) { + src := `package foo + var Unused = 5 + var control int + var _, A = f(Unused) + func Main() int { + return control + } + func f(int) (int, int) { + control = 5 + return 1, 2 + }` + eval(t, src, big.NewInt(5)) + }) + t.Run("argument of locally called function", func(t *testing.T) { + src := `package foo + var Unused = 5 + func Main() int { + var _, a = f(Unused) + return a + } + func f(i int) (int, int) { + return i, i + }` + eval(t, src, big.NewInt(5)) + }) + t.Run("used in globally called defer", func(t *testing.T) { + src := `package foo + var control1, control2 int + var Unused = 5 + var _ = f() + func Main() int { + return control1 + control2 + } + func f() int { + control1 = 1 + defer func(){ + control2 = Unused + }() + return 2 + }` + eval(t, src, big.NewInt(6)) + }) + t.Run("used in locally called defer", func(t *testing.T) { + src := `package foo + var control1, control2 int + var Unused = 5 + func Main() int { + _ = f() + return control1 + control2 + } + func f() int { + control1 = 1 + defer func(){ + control2 = Unused + }() + return 2 + }` + eval(t, src, big.NewInt(6)) + }) + t.Run("imported", func(t *testing.T) { + t.Run("init by func call", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar" + func Main() int { + return globalvar.Default + }` + eval(t, src, big.NewInt(0)) + checkInstrCount(t, src, 1 /* Default */, 1 /* f */, 0, 0) + }) + t.Run("nested var call", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/nested1" + func Main() int { + return nested1.C + }` + eval(t, src, big.NewInt(81)) + checkInstrCount(t, src, 6 /* dependant vars of nested1.C */, 3, 1, 1) + }) + t.Run("nested func call", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/funccall" + func Main() int { + return funccall.F() + }` + eval(t, src, big.NewInt(56)) + checkInstrCount(t, src, 2 /* nested2.Argument + nested1.Argument */, -1, -1, -1) + }) + t.Run("nested method call", func(t *testing.T) { + src := `package foo + import "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/funccall" + func Main() int { + return funccall.GetAge() + }` + eval(t, src, big.NewInt(24)) + checkInstrCount(t, src, 3, /* nested3.Anna + nested2.Argument + nested3.Argument */ + 5, /* funccall.GetAge() + Anna.GetAge() + nested1.f + nested1.f + nested2.f */ + 2 /* nested1.f + nested2.f */, 0) + }) + }) +} + func TestChangeGlobal(t *testing.T) { src := `package foo var a int @@ -360,19 +907,18 @@ func TestUnderscoreGlobalVarDontEmitCode(t *testing.T) { _, B, _ = 4, 5, 6 _, C, _ = f(A, B) ) - var D = 7 // unused but named, so the code is expected + var D = 7 // named unused, after global codegen optimisation no code expected func Main() int { return 1 } func f(a, b int) (int, int, int) { return 8, 9, 10 }` - eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{4}}, // sslot for A, B, C, D + eval(t, src, big.NewInt(1), []interface{}{opcode.INITSSLOT, []byte{2}}, // sslot for A, B opcode.PUSH2, opcode.STSFLD0, // store A opcode.PUSH5, opcode.STSFLD1, // store B - opcode.LDSFLD0, opcode.LDSFLD1, opcode.SWAP, []interface{}{opcode.CALL, []byte{10}}, // evaluate f - opcode.DROP, opcode.STSFLD2, opcode.DROP, // store C - opcode.PUSH7, opcode.STSFLD3, opcode.RET, // store D + opcode.LDSFLD0, opcode.LDSFLD1, opcode.SWAP, []interface{}{opcode.CALL, []byte{8}}, // evaluate f(A,B) + opcode.DROP, opcode.DROP, opcode.DROP, opcode.RET, // drop result of f(A,B) opcode.PUSH1, opcode.RET, // Main []interface{}{opcode.INITSLOT, []byte{0, 2}}, opcode.PUSH10, opcode.PUSH9, opcode.PUSH8, opcode.RET) // f } diff --git a/pkg/compiler/inline_test.go b/pkg/compiler/inline_test.go index fb4125543..ad783fe2c 100644 --- a/pkg/compiler/inline_test.go +++ b/pkg/compiler/inline_test.go @@ -3,6 +3,7 @@ package compiler_test import ( "fmt" "math/big" + "os" "strings" "testing" @@ -12,8 +13,12 @@ import ( ) func checkCallCount(t *testing.T, src string, expectedCall, expectedInitSlot, expectedLocalsMain int) { - v, sp, _ := vmAndCompileInterop(t, src) + checkInstrCount(t, src, -1, expectedCall, expectedInitSlot, expectedLocalsMain) +} +func checkInstrCount(t *testing.T, src string, expectedSSlotCount, expectedCall, expectedInitSlot, expectedLocalsMain int) { + v, sp, _ := vmAndCompileInterop(t, src) + v.PrintOps(os.Stdout) mainStart := -1 for _, m := range sp.info.Methods { if m.Name.Name == "main" { @@ -29,6 +34,14 @@ func checkCallCount(t *testing.T, src string, expectedCall, expectedInitSlot, ex for op, param, err := ctx.Next(); ; op, param, err = ctx.Next() { require.NoError(t, err) switch op { + case opcode.INITSSLOT: + if expectedSSlotCount == -1 { + continue + } + if expectedSSlotCount == 0 { + t.Fatalf("no INITSSLOT expected, found at %d with %d cells", ctx.IP(), param[0]) + } + require.Equal(t, expectedSSlotCount, int(param[0])) case opcode.CALL, opcode.CALLL: actualCall++ case opcode.INITSLOT: @@ -41,8 +54,12 @@ func checkCallCount(t *testing.T, src string, expectedCall, expectedInitSlot, ex break } } - require.Equal(t, expectedCall, actualCall) - require.True(t, expectedInitSlot == actualInitSlot) + if expectedCall != -1 { + require.Equal(t, expectedCall, actualCall) + } + if expectedInitSlot != -1 { + require.Equal(t, expectedInitSlot, actualInitSlot) + } } func TestInline(t *testing.T) { diff --git a/pkg/compiler/testdata/foo/foo.go b/pkg/compiler/testdata/foo/foo.go index ee4aef007..e81b0e7b9 100644 --- a/pkg/compiler/testdata/foo/foo.go +++ b/pkg/compiler/testdata/foo/foo.go @@ -5,7 +5,7 @@ func NewBar() int { return 10 } -// Dummy is dummy constant. +// Dummy is dummy variable. var Dummy = 1 // Foo is a type. diff --git a/pkg/compiler/testdata/globalvar/funccall/main.go b/pkg/compiler/testdata/globalvar/funccall/main.go new file mode 100644 index 000000000..9bd194caa --- /dev/null +++ b/pkg/compiler/testdata/globalvar/funccall/main.go @@ -0,0 +1,18 @@ +package funccall + +import ( + "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/nested1" + "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/nested2" + alias "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/nested3" +) + +// F should be called from the main package to check usage analyzer against +// nested constructions handling. +func F() int { + return nested1.F(nested2.Argument + alias.Argument) +} + +// GetAge calls method on the global struct. +func GetAge() int { + return alias.Anna.GetAge() +} diff --git a/pkg/compiler/testdata/globalvar/main.go b/pkg/compiler/testdata/globalvar/main.go new file mode 100644 index 000000000..5375f6ae6 --- /dev/null +++ b/pkg/compiler/testdata/globalvar/main.go @@ -0,0 +1,14 @@ +package globalvar + +// Unused shouldn't produce any initialization code if it's not used anywhere. +var Unused = 3 + +// Default is initialized by default value. +var Default int + +// A initialized by function call, thus the initialization code should always be emitted. +var A = f() + +func f() int { + return 5 +} diff --git a/pkg/compiler/testdata/globalvar/nested1/main.go b/pkg/compiler/testdata/globalvar/nested1/main.go new file mode 100644 index 000000000..64378bf03 --- /dev/null +++ b/pkg/compiler/testdata/globalvar/nested1/main.go @@ -0,0 +1,29 @@ +package nested1 + +import ( + "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/nested2" + alias "github.com/nspcc-dev/neo-go/pkg/compiler/testdata/globalvar/nested3" +) + +// Unused shouldn't produce any code if unused. +var Unused = 11 + +// A should produce call to f and should not be DROPped if C is used. It uses +// aliased package var as an argument to check analizator. +var A = f(alias.Argument) + +// B should produce call to f and be DROPped if unused. It uses foreign package var as an argument +// to check analizator. +var B = f(nested2.Argument) + +// C shouldn't produce any code if unused. It uses +var C = A + nested2.A + nested2.Unique + +func f(i int) int { + return i +} + +// F is used for nested calls check. +func F(i int) int { + return i +} diff --git a/pkg/compiler/testdata/globalvar/nested2/main.go b/pkg/compiler/testdata/globalvar/nested2/main.go new file mode 100644 index 000000000..6ae1205f5 --- /dev/null +++ b/pkg/compiler/testdata/globalvar/nested2/main.go @@ -0,0 +1,20 @@ +package nested2 + +// Unused shouldn't produce any code if unused. +var Unused = 21 + +// Argument is an argument used from external package to call nested1.f. +var Argument = 22 + +// A has the same name as nested1.A. +var A = 23 + +// B should produce call to f and be DROPped if unused. +var B = f() + +// Unique has unique name. +var Unique = 24 + +func f() int { + return 25 +} diff --git a/pkg/compiler/testdata/globalvar/nested3/main.go b/pkg/compiler/testdata/globalvar/nested3/main.go new file mode 100644 index 000000000..2cbdc4e71 --- /dev/null +++ b/pkg/compiler/testdata/globalvar/nested3/main.go @@ -0,0 +1,18 @@ +package nested3 + +// Argument is used as a function argument. +var Argument = 34 + +// Anna is used to check struct-related usage analyzer logic (calls to methods +// and fields). +var Anna = Person{Age: 24} + +// Person is an auxiliary structure containing simple field. +type Person struct { + Age int +} + +// GetAge is used to check method calls inside usage analyzer. +func (p Person) GetAge() int { + return p.Age +} From 7f613e63aad1ec8115746fbb6e8b73446231d36b Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 30 Aug 2022 14:20:18 +0300 Subject: [PATCH 5/5] compiler: add test for #2661 --- pkg/compiler/global_test.go | 39 +++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/pkg/compiler/global_test.go b/pkg/compiler/global_test.go index 46ff61535..7b972dd5d 100644 --- a/pkg/compiler/global_test.go +++ b/pkg/compiler/global_test.go @@ -628,18 +628,33 @@ func TestUnusedOptimizedGlobalVar(t *testing.T) { } func TestChangeGlobal(t *testing.T) { - src := `package foo - var a int - func Main() int { - setLocal() - set42() - setLocal() - return a - } - func set42() { a = 42 } - func setLocal() { a := 10; _ = a }` - - eval(t, src, big.NewInt(42)) + t.Run("from Main", func(t *testing.T) { + src := `package foo + var a int + func Main() int { + setLocal() + set42() + setLocal() + return a + } + func set42() { a = 42 } + func setLocal() { a := 10; _ = a }` + eval(t, src, big.NewInt(42)) + }) + t.Run("from other global", func(t *testing.T) { + t.Skip("see https://github.com/nspcc-dev/neo-go/issues/2661") + src := `package foo + var A = f() + var B int + func Main() int { + return B + } + func f() int { + B = 3 + return B + }` + eval(t, src, big.NewInt(3)) + }) } func TestMultiDeclaration(t *testing.T) {