From 1dcbdb011a50cd1acfed5a2483a8c75a475f804c Mon Sep 17 00:00:00 2001
From: Anna Shaleva <shaleva.ann@nspcc.ru>
Date: Mon, 15 Aug 2022 13:15:24 +0300
Subject: [PATCH] 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) {