fix(vm): preserve Lua modulo semantics for negative operands

doc_test.go

@@ -1,22 +1,37 @@
 package lua_test
 
 import (
+	"fmt"
+
 	"github.com/Shopify/go-lua"
 )
 
 // This example receives a variable number of numerical arguments and returns their average and sum.
-func ExampleFunction(l *lua.State) int {
-	n := l.Top() // Number of arguments.
-	var sum float64
-	for i := 1; i <= n; i++ {
-		f, ok := l.ToNumber(i)
-		if !ok {
-			l.PushString("incorrect argument")
-			l.Error()
+func ExampleFunction() {
+	averageAndSum := func(l *lua.State) int {
+		n := l.Top() // Number of arguments.
+		var sum float64
+		for i := 1; i <= n; i++ {
+			f, ok := l.ToNumber(i)
+			if !ok {
+				l.PushString("incorrect argument")
+				l.Error()
+			}
+			sum += f
 		}
-		sum += f
+		l.PushNumber(sum / float64(n)) // First result.
+		l.PushNumber(sum)              // Second result.
+		return 2                       // Result count.
 	}
-	l.PushNumber(sum / float64(n)) // First result.
-	l.PushNumber(sum)              // Second result.
-	return 2                       // Result count.
+
+	l := lua.NewState()
+	l.PushGoFunction(averageAndSum)
+	l.PushNumber(1)
+	l.PushNumber(2)
+	l.PushNumber(3)
+	l.Call(3, 2)
+	average, _ := l.ToNumber(-2)
+	sum, _ := l.ToNumber(-1)
+	fmt.Println(average, sum)
+	// Output: 2 6
 }

pow_test.go

@@ -0,0 +1,101 @@
+package lua
+
+import (
+	"fmt"
+	"math"
+	"testing"
+)
+
+func TestPowerConsistentAcrossEvaluationPaths(t *testing.T) {
+	engines := []struct {
+		name string
+		run  func(*State)
+	}{
+		{"function table", (*State).executeFunctionTable},
+		{"switch", (*State).executeSwitch},
+	}
+	cases := []struct {
+		base, exponent             string
+		baseNumber, exponentNumber float64
+	}{
+		{"10", "-33", 10, -33},
+		{"10", "33", 10, 33},
+		{"10", "100", 10, 100},
+		{"10", "308", 10, 308},
+		{"10", "309", 10, 309},
+		{"10", "-323", 10, -323},
+		{"10", "-324", 10, -324},
+		{"-2", "3", -2, 3},
+		{"-2", "-3", -2, -3},
+		{"-2", "0.5", -2, 0.5},
+		{"0", "0", 0, 0},
+		{"0", "-3", 0, -3},
+		{"-0.0", "3", math.Copysign(0, -1), 3},
+		{"-0.0", "-3", math.Copysign(0, -1), -3},
+		{"-0.0", "2", math.Copysign(0, -1), 2},
+		{"-0.0", "-2", math.Copysign(0, -1), -2},
+		{"2", "0.5", 2, 0.5},
+	}
+	for _, engine := range engines {
+		for _, tc := range cases {
+			t.Run(engine.name+"/"+tc.base+"^"+tc.exponent, func(t *testing.T) {
+				l := NewState()
+				OpenLibraries(l)
+				source := fmt.Sprintf(`
+local base, exponent = (%s), (%s)
+local folded = (%s)^(%s)
+local runtime = base^exponent
+local coerced = %q^%q
+local library = math.pow(base, exponent)
+local keyMatches = true
+if folded == folded then
+  local keys = {[folded] = "found"}
+  keyMatches = keys[runtime] == "found" and keys[coerced] == "found" and keys[library] == "found"
+end
+return folded, runtime, coerced, library, keyMatches
+`, tc.base, tc.exponent, tc.base, tc.exponent, tc.base, tc.exponent)
+				if err := LoadString(l, source); err != nil {
+					t.Fatal(err)
+				}
+				if err := l.protectedCall(func() {
+					if l.preCall(l.top-1, MultipleReturns) {
+						t.Fatal("expected a Lua function")
+					}
+					engine.run(l)
+				}, l.top-1, 0); err != nil {
+					t.Fatal(err)
+				}
+				// Use the runtime's numeric primitive, not a decimal approximation
+				// or the result from a different host math library.
+				want := math.Pow(tc.baseNumber, tc.exponentNumber)
+				for index, name := range []string{"folded", "runtime", "coerced", "math.pow"} {
+					got, ok := l.ToNumber(index + 1)
+					if !ok || !samePowerNumber(got, want) {
+						t.Errorf("%s = %g (%016x), want %g (%016x)", name, got, math.Float64bits(got), want, math.Float64bits(want))
+					}
+				}
+				if !l.ToBoolean(5) {
+					t.Error("folded and runtime powers did not address the same table key")
+				}
+				for _, coerce := range []bool{false, true} {
+					api := NewState()
+					if coerce {
+						api.PushString(tc.base)
+						api.PushString(tc.exponent)
+					} else {
+						api.PushNumber(tc.baseNumber)
+						api.PushNumber(tc.exponentNumber)
+					}
+					api.Arith(OpPow)
+					if got, ok := api.ToNumber(-1); !ok || !samePowerNumber(got, want) {
+						t.Errorf("Arith(OpPow), coerce=%v: %g, want %g", coerce, got, want)
+					}
+				}
+			})
+		}
+	}
+}
+
+func samePowerNumber(a, b float64) bool {
+	return math.Float64bits(a) == math.Float64bits(b) || math.IsNaN(a) && math.IsNaN(b)
+}

types.go

@@ -238,10 +238,6 @@ func arith(op Operator, v1, v2 float64) float64 {
 	case OpMod:
 		return v1 - math.Floor(v1/v2)*v2
 	case OpPow:
-		// Golang bug: math.Pow(10.0, 33.0) is incorrect by 1 bit.
-		if v1 == 10.0 && float64(int(v2)) == v2 {
-			return math.Pow10(int(v2))
-		}
 		return math.Pow(v1, v2)
 	case OpUnaryMinus:
 		return -v1

vm.go

@@ -526,7 +526,7 @@ func init() {
 			c := e.k(i.c())
 			if nb, ok := b.(float64); ok {
 				if nc, ok := c.(float64); ok {
-					e.frame[i.a()] = math.Mod(nb, nc)
+					e.frame[i.a()] = arith(OpMod, nb, nc)
 					if e.hooked() {
 						e.hook()
 					}
@@ -1083,7 +1083,7 @@ func (l *State) executeSwitch() {
 			c := k(i.c(), constants, frame)
 			if nb, ok := b.(float64); ok {
 				if nc, ok := c.(float64); ok {
-					frame[i.a()] = math.Mod(nb, nc)
+					frame[i.a()] = arith(OpMod, nb, nc)
 					break
 				}
 			}

vm_test.go

@@ -51,6 +51,24 @@ func TestProtectedCall(t *testing.T) {
 	l.Call(0, 0)
 }
 
+func TestModuloNegativeOperands(t *testing.T) {
+	l := NewState()
+	OpenLibraries(l)
+
+	if err := DoString(l, `local a,b,c,d=-3,2,3,-2
+		return -3%2, a%b, "-3"%b, c%d, math.fmod(a,b)`); err != nil {
+		t.Fatal(err)
+	}
+
+	want := []float64{1, 1, 1, -1, -1}
+	for i, expected := range want {
+		got, ok := l.ToNumber(i + 1)
+		if !ok || got != expected {
+			t.Errorf("result %d = %v (number: %t), want %v", i+1, got, ok, expected)
+		}
+	}
+}
+
 func TestLua(t *testing.T) {
 	tests := []struct {
 		name    string