From b461a6ab63cc2c013a1cbb0cf2199ee9126d0130 Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <evgeniy@nspcc.ru>
Date: Fri, 28 Feb 2020 17:18:42 +0300
Subject: [PATCH] compiler: do not short-circuit in complex conditions

Current implementation of short-circuting is just plain wrong
as it uses `last` or `before-last` labels which meaning depend
on context. It doesn't even handle simple assignements like
`a := x == 1 && y == 2`.

This commit makes all jumps in such conditions local
and adds tests.

Closes #699, #700.
---
 pkg/compiler/codegen.go  | 16 +++++++++++--
 pkg/compiler/for_test.go | 50 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/pkg/compiler/codegen.go b/pkg/compiler/codegen.go
index f7f56e382..0f0f4563c 100644
--- a/pkg/compiler/codegen.go
+++ b/pkg/compiler/codegen.go
@@ -505,15 +505,27 @@ func (c *codegen) Visit(node ast.Node) ast.Visitor {
 	case *ast.BinaryExpr:
 		switch n.Op {
 		case token.LAND:
+			next := c.newLabel()
+			end := c.newLabel()
 			ast.Walk(c, n.X)
-			emit.Jmp(c.prog.BinWriter, opcode.JMPIFNOT, uint16(len(c.l)-1))
+			emit.Jmp(c.prog.BinWriter, opcode.JMPIF, next)
+			emit.Opcode(c.prog.BinWriter, opcode.PUSHF)
+			emit.Jmp(c.prog.BinWriter, opcode.JMP, end)
+			c.setLabel(next)
 			ast.Walk(c, n.Y)
+			c.setLabel(end)
 			return nil
 
 		case token.LOR:
+			next := c.newLabel()
+			end := c.newLabel()
 			ast.Walk(c, n.X)
-			emit.Jmp(c.prog.BinWriter, opcode.JMPIF, uint16(len(c.l)-3))
+			emit.Jmp(c.prog.BinWriter, opcode.JMPIFNOT, next)
+			emit.Opcode(c.prog.BinWriter, opcode.PUSHT)
+			emit.Jmp(c.prog.BinWriter, opcode.JMP, end)
+			c.setLabel(next)
 			ast.Walk(c, n.Y)
+			c.setLabel(end)
 			return nil
 
 		default:
diff --git a/pkg/compiler/for_test.go b/pkg/compiler/for_test.go
index cb9f1f536..56f34da3c 100644
--- a/pkg/compiler/for_test.go
+++ b/pkg/compiler/for_test.go
@@ -1,6 +1,7 @@
 package compiler_test
 
 import (
+	"fmt"
 	"math/big"
 	"strings"
 	"testing"
@@ -704,3 +705,52 @@ func TestForLoopRangeCompilerError(t *testing.T) {
 	_, err := compiler.Compile(strings.NewReader(src))
 	require.Error(t, err)
 }
+
+func TestForLoopComplexConditions(t *testing.T) {
+	src := `
+	package foo
+	func Main() int {
+		var ok bool
+		_ = ok
+		i := 0
+		j := 0
+		%s
+		for %s {
+			i++
+			j++
+			%s
+		}
+		return i
+	}`
+
+	tests := []struct {
+		Name   string
+		Cond   string
+		Assign string
+		Result int64
+	}{
+		{Cond: "i < 3 && j < 2", Result: 2},
+		{Cond: "i < 3 || j < 2", Result: 3},
+		{Cond: "i < 3 && (j < 2 || i < 1)", Result: 2},
+		{Cond: "i < 3 && (j < 2 && i < 1)", Result: 1},
+		{Cond: "(i < 1 || j < 3) && (i < 3 || j < 1)", Result: 3},
+		{Cond: "(i < 2 && j < 4) || (i < 4 && j < 2)", Result: 2},
+		{Cond: "ok", Assign: "ok = i < 3 && j < 2", Result: 2},
+		{Cond: "ok", Assign: "ok = i < 3 || j < 2", Result: 3},
+		{Cond: "ok", Assign: "ok = i < 3 && (j < 2 || i < 1)", Result: 2},
+		{Cond: "ok", Assign: "ok = i < 3 && (j < 2 && i < 1)", Result: 1},
+		{Cond: "ok", Assign: "ok = (i < 1 || j < 3) && (i < 3 || j < 1)", Result: 3},
+		{Cond: "ok", Assign: "ok = (i < 2 && j < 4) || (i < 4 && j < 2)", Result: 2},
+	}
+
+	for _, tc := range tests {
+		name := tc.Cond
+		if tc.Assign != "" {
+			name = tc.Assign
+		}
+		t.Run(name, func(t *testing.T) {
+			s := fmt.Sprintf(src, tc.Assign, tc.Cond, tc.Assign)
+			eval(t, s, big.NewInt(tc.Result))
+		})
+	}
+}