From 91b36657d6093bd76e4abac8e64468bf62bd458d Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Fri, 19 Aug 2022 14:54:42 +0300 Subject: [PATCH] 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) {