internal/shader: bug fix: % should be valid only for integers

Closes hajimehoshi#1947
hermunn · Jan 11, 2022 · 83bd077 · 83bd077
1 parent ca3f84f
commit 83bd077
Show file tree

Hide file tree

Showing 3 changed files with 158 additions and 3 deletions.
diff --git a/internal/shader/expr.go b/internal/shader/expr.go
@@ -29,6 +29,22 @@ func canTruncateToInteger(v gconstant.Value) bool {
 	return gconstant.ToInt(v).Kind() != gconstant.Unknown
 }
 
+func goConstantKindString(k gconstant.Kind) string {
+	switch k {
+	case gconstant.Bool:
+		return "bool"
+	case gconstant.String:
+		return "string"
+	case gconstant.Int:
+		return "int"
+	case gconstant.Float:
+		return "float"
+	case gconstant.Complex:
+		return "complex"
+	}
+	return "unknown"
+}
+
 var textureVariableRe = regexp.MustCompile(`\A__t(\d+)\z`)
 
 func (cs *compileState) parseExpr(block *block, expr ast.Expr, markLocalVariableUsed bool) ([]shaderir.Expr, []shaderir.Type, []shaderir.Stmt, bool) {
@@ -94,6 +110,18 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr, markLocalVariable
 				v = gconstant.MakeBool(gconstant.Compare(lhs[0].Const, op, rhs[0].Const))
 				t = shaderir.Type{Main: shaderir.Bool}
 			default:
+				if op == token.REM {
+					if lhs[0].Const.Kind() != gconstant.Int || rhs[0].Const.Kind() != gconstant.Int {
+						var wrongTypeName string
+						if lhs[0].Const.Kind() != gconstant.Int {
+							wrongTypeName = goConstantKindString(lhs[0].Const.Kind())
+						} else {
+							wrongTypeName = goConstantKindString(rhs[0].Const.Kind())
+						}
+						cs.addError(e.Pos(), fmt.Sprintf("invalid operation: operator %% not defined on untyped %s", wrongTypeName))
+						return nil, nil, nil, false
+					}
+				}
 				v = gconstant.BinaryOp(lhs[0].Const, op, rhs[0].Const)
 				if v.Kind() == gconstant.Float {
 					t = shaderir.Type{Main: shaderir.Float}
@@ -174,6 +202,20 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr, markLocalVariable
 			return nil, nil, nil, false
 		}
 
+		if op == shaderir.ModOp {
+			// TODO: What about ivec?
+			if lhst.Main != shaderir.Int || rhst.Main != shaderir.Int {
+				var wrongType shaderir.Type
+				if lhst.Main != shaderir.Int {
+					wrongType = lhst
+				} else {
+					wrongType = rhst
+				}
+				cs.addError(e.Pos(), fmt.Sprintf("invalid operation: operator %% not defined on %s", wrongType.String()))
+				return nil, nil, nil, false
+			}
+		}
+
 		return []shaderir.Expr{
 			{
 				Type:  shaderir.Binary,

diff --git a/internal/shader/stmt.go b/internal/shader/stmt.go
@@ -75,19 +75,32 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 				op = shaderir.ModOp
 			}
 
-			rhs, _, ss, ok := cs.parseExpr(block, stmt.Rhs[0], true)
+			rhs, rts, ss, ok := cs.parseExpr(block, stmt.Rhs[0], true)
 			if !ok {
 				return nil, false
 			}
 			stmts = append(stmts, ss...)
 
-			lhs, ts, ss, ok := cs.parseExpr(block, stmt.Lhs[0], true)
+			lhs, lts, ss, ok := cs.parseExpr(block, stmt.Lhs[0], true)
 			if !ok {
 				return nil, false
 			}
 			stmts = append(stmts, ss...)
 
-			if rhs[0].Type == shaderir.NumberExpr && ts[0].Main == shaderir.Int {
+			if op == shaderir.ModOp {
+				if lts[0].Main != shaderir.Int || rts[0].Main != shaderir.Int {
+					var wrongType shaderir.Type
+					if lts[0].Main != shaderir.Int {
+						wrongType = lts[0]
+					} else {
+						wrongType = rts[0]
+					}
+					cs.addError(stmt.Pos(), fmt.Sprintf("invalid operation: operator %% not defined on %s", wrongType.String()))
+					return nil, false
+				}
+			}
+
+			if rhs[0].Type == shaderir.NumberExpr && rts[0].Main == shaderir.Int {
 				if !cs.forceToInt(stmt, &rhs[0]) {
 					return nil, false
 				}

diff --git a/shader_test.go b/shader_test.go
@@ -1269,3 +1269,103 @@ func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
 		t.Errorf("error must be non-nil but was nil")
 	}
 }
+
+// Issue #1947
+func TestShaderOperatorMod(t *testing.T) {
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2.0 % 0.5
+	return vec4(a)
+}`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2.0
+	b := 0.5
+	return vec4(a % b)
+}`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2
+	b := 0.5
+	return vec4(a % b)
+}`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2.5
+	b := 1
+	return vec4(a % b)
+}`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2
+	b := 1
+	return vec4(a % b)
+}`)); err != nil {
+		t.Error(err)
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2
+	return vec4(a % 1)
+}`)); err != nil {
+		t.Error(err)
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 1
+	return vec4(2 % a)
+}`)); err != nil {
+		t.Error(err)
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2
+	a %= 1
+	return vec4(a)
+}`)); err != nil {
+		t.Error(err)
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2
+	a %= 0.5
+	return vec4(a)
+}`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+
+	if _, err := ebiten.NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	a := 2.0
+	a %= 1
+	return vec4(a)
+}`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+}