From 83bd0772d4b6fb57ddd0fbcf73fd927a9a9aa6e1 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Tue, 11 Jan 2022 23:33:40 +0900 Subject: [PATCH] internal/shader: bug fix: % should be valid only for integers Closes #1947 --- internal/shader/expr.go | 42 +++++++++++++++++ internal/shader/stmt.go | 19 ++++++-- shader_test.go | 100 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 3 deletions(-) diff --git a/internal/shader/expr.go b/internal/shader/expr.go index febd2aa8e937..594b027cddf1 100644 --- 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 index aae0a6be0d66..508635d2b9af 100644 --- 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 index 7ebe64e8eae0..d30ac41810eb 100644 --- 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") + } +}