From 15fe7158fd250db1d7a2956c772d354d1a628c51 Mon Sep 17 00:00:00 2001 From: Hajime Hoshi Date: Fri, 8 Apr 2022 22:44:37 +0900 Subject: [PATCH] internal/shader: implement strict type checks in assignments Closes #1972 --- internal/shader/shader.go | 18 ++- internal/shader/stmt.go | 34 +++++- internal/shader/syntax_test.go | 124 +++++++++++++++++++++ internal/shader/testdata/array.expected.vs | 2 +- internal/shader/testdata/array.go | 2 +- 5 files changed, 170 insertions(+), 10 deletions(-) diff --git a/internal/shader/shader.go b/internal/shader/shader.go index 05c98c2ea..1b5059ecd 100644 --- a/internal/shader/shader.go +++ b/internal/shader/shader.go @@ -336,6 +336,7 @@ func (cs *compileState) parseDecl(b *block, d ast.Decl) ([]shaderir.Stmt, bool) if !ok { return nil, false } + stmts = append(stmts, ss...) if b == &cs.global { // TODO: Should rhs be ignored? @@ -473,7 +474,7 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl init := vs.Values[i] - es, origts, ss, ok := s.parseExpr(block, init, true) + es, rts, ss, ok := s.parseExpr(block, init, true) if !ok { return nil, nil, nil, false } @@ -481,7 +482,7 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl if t.Main == shaderir.None { ts, ok := s.functionReturnTypes(block, init) if !ok { - ts = origts + ts = rts } if len(ts) > 1 { s.addError(vs.Pos(), fmt.Sprintf("the numbers of lhs and rhs don't match")) @@ -498,6 +499,12 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl } } + for i, rt := range rts { + if !canAssign(&es[i], &t, &rt) { + s.addError(vs.Pos(), fmt.Sprintf("cannot use type %s as type %s in variable declaration", rt.String(), t.String())) + } + } + inits = append(inits, es...) stmts = append(stmts, ss...) @@ -526,10 +533,15 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl } } } - if len(inittypes) > 0 { + + if t.Main == shaderir.None && len(inittypes) > 0 { t = inittypes[i] } + if !canAssign(&initexprs[i], &t, &inittypes[i]) { + s.addError(vs.Pos(), fmt.Sprintf("cannot use type %s as type %s in variable declaration", inittypes[i].String(), t.String())) + } + // Add the same initexprs for each variable. inits = append(inits, initexprs...) } diff --git a/internal/shader/stmt.go b/internal/shader/stmt.go index 2aefdd235..0e89f8831 100644 --- a/internal/shader/stmt.go +++ b/internal/shader/stmt.go @@ -562,7 +562,7 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r for i, e := range lhs { if len(lhs) == len(rhs) { // Prase RHS first for the order of the statements. - r, origts, ss, ok := cs.parseExpr(block, rhs[i], true) + r, rts, ss, ok := cs.parseExpr(block, rhs[i], true) if !ok { return nil, false } @@ -580,7 +580,7 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r } ts, ok := cs.functionReturnTypes(block, rhs[i]) if !ok { - ts = origts + ts = rts } if len(ts) > 1 { cs.addError(pos, fmt.Sprintf("single-value context and multiple-value context cannot be mixed")) @@ -599,7 +599,7 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r return nil, false } - l, _, ss, ok := cs.parseExpr(block, lhs[i], false) + l, lts, ss, ok := cs.parseExpr(block, lhs[i], false) if !ok { return nil, false } @@ -646,6 +646,13 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r } } + for i := range lts { + if !canAssign(&r[i], <s[i], &rts[i]) { + cs.addError(pos, fmt.Sprintf("cannot use type %s as type %s in variable declaration", rts[i].String(), lts[i].String())) + return nil, false + } + } + if len(lhs) == 1 { stmts = append(stmts, shaderir.Stmt{ Type: shaderir.Assign, @@ -653,7 +660,7 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r }) } else { // For variable swapping, use temporary variables. - t := origts[0] + t := rts[0] if t.Main == shaderir.None { t = toDefaultType(r[0].Const) } @@ -716,7 +723,7 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r block.addNamedLocalVariable(name, t, e.Pos()) } - l, _, ss, ok := cs.parseExpr(block, lhs[i], false) + l, lts, ss, ok := cs.parseExpr(block, lhs[i], false) if !ok { return nil, false } @@ -727,6 +734,13 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r } allblank = false + for i, lt := range lts { + if !canAssign(&rhsExprs[i], <, &rhsTypes[i]) { + cs.addError(pos, fmt.Sprintf("cannot use type %s as type %s in variable declaration", rhsTypes[i].String(), lt.String())) + return nil, false + } + } + stmts = append(stmts, shaderir.Stmt{ Type: shaderir.Assign, Exprs: []shaderir.Expr{l[0], rhsExprs[i]}, @@ -754,3 +768,13 @@ func toDefaultType(v gconstant.Value) shaderir.Type { // TODO: Should this be an error? return shaderir.Type{} } + +func canAssign(re *shaderir.Expr, lt *shaderir.Type, rt *shaderir.Type) bool { + if lt.Equal(rt) { + return true + } + if re.Type == shaderir.NumberExpr && (lt.Main == shaderir.Float || lt.Main == shaderir.Int || lt.Main == shaderir.Bool) { + return true + } + return false +} diff --git a/internal/shader/syntax_test.go b/internal/shader/syntax_test.go index 9f2df3db3..a1dee303c 100644 --- a/internal/shader/syntax_test.go +++ b/internal/shader/syntax_test.go @@ -1183,3 +1183,127 @@ func Fragment(position vec4, texCoord vec2, color vec4) vec4 { t.Error(err) } } + +// Issue #1972 +func TestSyntaxType(t *testing.T) { + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x vec2 = vec3(0) + _ = x + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x, y vec2 = vec2(0), vec3(0) + _, _ = x, y + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x vec2 + x = vec3(0) + _ = x + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x, y vec2 + x, y = vec2(0), vec3(0) + _ = x + _ = y + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x vec2 + x = 0 + _ = x + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Foo() (vec3, vec3) { + return vec3(0), vec3(1) +} + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x, y vec2 = Foo() + _ = x + _ = y + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Foo() (vec3, vec3) { + return vec3(0), vec3(1) +} + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var x, y vec2 + x, y = Foo() + _ = x + _ = y + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } +} + +// Issue #1972 +func TestSyntaxTypeBlankVar(t *testing.T) { + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var _ vec2 = vec3(0) + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var _, _ vec2 = vec2(0), vec3(0) + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } + if _, err := compileToIR([]byte(`package main + +func Foo() (vec3, vec3) { + return vec3(0), vec3(1) +} + +func Fragment(position vec4, texCoord vec2, color vec4) vec4 { + var _, _ vec2 = Foo() + return color +} +`)); err == nil { + t.Errorf("error must be non-nil but was nil") + } +} diff --git a/internal/shader/testdata/array.expected.vs b/internal/shader/testdata/array.expected.vs index 85dfa7f02..0b1cf4699 100644 --- a/internal/shader/testdata/array.expected.vs +++ b/internal/shader/testdata/array.expected.vs @@ -22,7 +22,7 @@ void F1(out vec2 l0[2]) { (l1)[0] = vec2(1.0); l2[0] = l1[0]; l2[1] = l1[1]; - ((l2)[1]).y = vec2(2.0); + (l2)[1] = vec2(2.0); l0[0] = l2[0]; l0[1] = l2[1]; return; diff --git a/internal/shader/testdata/array.go b/internal/shader/testdata/array.go index 8aa9c0ce1..591fed936 100644 --- a/internal/shader/testdata/array.go +++ b/internal/shader/testdata/array.go @@ -9,6 +9,6 @@ func Foo() [2]vec2 { func Bar() [2]vec2 { x := [2]vec2{vec2(1)} - x[1].y = vec2(2) + x[1] = vec2(2) return x }