shader: Check unused local variables

Fixes #1328

internal/shader/expr.go

@@ -31,7 +31,7 @@ func canTruncateToInteger(v gconstant.Value) bool {
 
 var textureVariableRe = regexp.MustCompile(`\A__t(\d+)\z`)
 
-func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr, []shaderir.Type, []shaderir.Stmt, bool) {
+func (cs *compileState) parseExpr(block *block, expr ast.Expr, markLocalVariableUsed bool) ([]shaderir.Expr, []shaderir.Type, []shaderir.Stmt, bool) {
 	switch e := expr.(type) {
 	case *ast.BasicLit:
 		switch e.Kind {
@@ -57,7 +57,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 		var stmts []shaderir.Stmt
 
 		// Prase LHS first for the order of the statements.
-		lhs, ts, ss, ok := cs.parseExpr(block, e.X)
+		lhs, ts, ss, ok := cs.parseExpr(block, e.X, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}
@@ -68,7 +68,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 		stmts = append(stmts, ss...)
 		lhst := ts[0]
 
-		rhs, ts, ss, ok := cs.parseExpr(block, e.Y)
+		rhs, ts, ss, ok := cs.parseExpr(block, e.Y, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}
@@ -192,7 +192,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 
 		// Parse the argument first for the order of the statements.
 		for _, a := range e.Args {
-			es, ts, ss, ok := cs.parseExpr(block, a)
+			es, ts, ss, ok := cs.parseExpr(block, a, markLocalVariableUsed)
 			if !ok {
 				return nil, nil, nil, false
 			}
@@ -206,7 +206,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 		}
 
 		// TODO: When len(ss) is not 0?
-		es, _, ss, ok := cs.parseExpr(block, e.Fun)
+		es, _, ss, ok := cs.parseExpr(block, e.Fun, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}
@@ -376,7 +376,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 				},
 			}, nil, nil, true
 		}
-		if i, t, ok := block.findLocalVariable(e.Name); ok {
+		if i, t, ok := block.findLocalVariable(e.Name, markLocalVariableUsed); ok {
 			return []shaderir.Expr{
 				{
 					Type:  shaderir.LocalVariable,
@@ -428,10 +428,10 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 		cs.addError(e.Pos(), fmt.Sprintf("unexpected identifier: %s", e.Name))
 
 	case *ast.ParenExpr:
-		return cs.parseExpr(block, e.X)
+		return cs.parseExpr(block, e.X, markLocalVariableUsed)
 
 	case *ast.SelectorExpr:
-		exprs, _, stmts, ok := cs.parseExpr(block, e.X)
+		exprs, _, stmts, ok := cs.parseExpr(block, e.X, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}
@@ -467,7 +467,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 		}, []shaderir.Type{t}, stmts, true
 
 	case *ast.UnaryExpr:
-		exprs, t, stmts, ok := cs.parseExpr(block, e.X)
+		exprs, t, stmts, ok := cs.parseExpr(block, e.X, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}
@@ -526,7 +526,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 
 		var stmts []shaderir.Stmt
 		for i, e := range e.Elts {
-			exprs, _, ss, ok := cs.parseExpr(block, e)
+			exprs, _, ss, ok := cs.parseExpr(block, e, markLocalVariableUsed)
 			if !ok {
 				return nil, nil, nil, false
 			}
@@ -568,7 +568,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 		var stmts []shaderir.Stmt
 
 		// Parse the index first
-		exprs, _, ss, ok := cs.parseExpr(block, e.Index)
+		exprs, _, ss, ok := cs.parseExpr(block, e.Index, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}
@@ -587,7 +587,7 @@ func (cs *compileState) parseExpr(block *block, expr ast.Expr) ([]shaderir.Expr,
 			idx.ConstType = shaderir.ConstTypeInt
 		}
 
-		exprs, ts, ss, ok := cs.parseExpr(block, e.X)
+		exprs, ts, ss, ok := cs.parseExpr(block, e.X, markLocalVariableUsed)
 		if !ok {
 			return nil, nil, nil, false
 		}

internal/shader/shader.go

@@ -83,11 +83,12 @@ type typ struct {
 }
 
 type block struct {
-	types  []typ
+	types      []typ
-	vars   []variable
+	vars       []variable
-	consts []constant
+	unusedVars map[int]token.Pos
-	pos    token.Pos
+	consts     []constant
-	outer  *block
+	pos        token.Pos
+	outer      *block
 
 	ir *shaderir.Block
 }
@@ -100,7 +101,22 @@ func (b *block) totalLocalVariableNum() int {
 	return c
 }
 
-func (b *block) findLocalVariable(name string) (int, shaderir.Type, bool) {
+func (b *block) addNamedLocalVariable(name string, typ shaderir.Type, pos token.Pos) {
+	b.vars = append(b.vars, variable{
+		name: name,
+		typ:  typ,
+	})
+	if name == "_" {
+		return
+	}
+	idx := len(b.vars) - 1
+	if b.unusedVars == nil {
+		b.unusedVars = map[int]token.Pos{}
+	}
+	b.unusedVars[idx] = pos
+}
+
+func (b *block) findLocalVariable(name string, markLocalVariableUsed bool) (int, shaderir.Type, bool) {
 	if name == "" || name == "_" {
 		panic("shader: variable name must be non-empty and non-underscore")
 	}
@@ -111,11 +127,14 @@ func (b *block) findLocalVariable(name string) (int, shaderir.Type, bool) {
 	}
 	for i, v := range b.vars {
 		if v.name == name {
+			if markLocalVariableUsed {
+				delete(b.unusedVars, i)
+			}
 			return idx + i, v.typ, true
 		}
 	}
 	if b.outer != nil {
-		return b.outer.findLocalVariable(name)
+		return b.outer.findLocalVariable(name, markLocalVariableUsed)
 	}
 	return 0, shaderir.Type{}, false
 }
@@ -422,7 +441,7 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl
 
 			init := vs.Values[i]
 
-			es, origts, ss, ok := s.parseExpr(block, init)
+			es, origts, ss, ok := s.parseExpr(block, init, true)
 			if !ok {
 				return nil, nil, nil, false
 			}
@@ -458,7 +477,7 @@ func (s *compileState) parseVariable(block *block, vs *ast.ValueSpec) ([]variabl
 
 				var ss []shaderir.Stmt
 				var ok bool
-				initexprs, inittypes, ss, ok = s.parseExpr(block, init)
+				initexprs, inittypes, ss, ok = s.parseExpr(block, init, true)
 				if !ok {
 					return nil, nil, nil, false
 				}
@@ -644,7 +663,7 @@ func (cs *compileState) parseFunc(block *block, d *ast.FuncDecl) (function, bool
 		}
 	}
 
-	b, ok := cs.parseBlock(block, d.Name.Name, d.Body.List, inParams, outParams)
+	b, ok := cs.parseBlock(block, d.Name.Name, d.Body.List, inParams, outParams, true)
 	if !ok {
 		return function{}, false
 	}
@@ -690,7 +709,7 @@ func (cs *compileState) parseFunc(block *block, d *ast.FuncDecl) (function, bool
 	}, true
 }
 
-func (cs *compileState) parseBlock(outer *block, fname string, stmts []ast.Stmt, inParams, outParams []variable) (*block, bool) {
+func (cs *compileState) parseBlock(outer *block, fname string, stmts []ast.Stmt, inParams, outParams []variable, checkLocalVariableUsage bool) (*block, bool) {
 	var vars []variable
 	if outer == &cs.global {
 		vars = make([]variable, 0, len(inParams)+len(outParams))
@@ -745,5 +764,12 @@ func (cs *compileState) parseBlock(outer *block, fname string, stmts []ast.Stmt,
 		block.ir.Stmts = append(block.ir.Stmts, ss...)
 	}
 
+	if checkLocalVariableUsage && len(block.unusedVars) > 0 {
+		for idx, pos := range block.unusedVars {
+			cs.addError(pos, fmt.Sprintf("local variable %s is not used", block.vars[idx].name))
+		}
+		return nil, false
+	}
+
 	return block, true
 }

internal/shader/stmt.go

@@ -75,13 +75,13 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 				op = shaderir.ModOp
 			}
 
-			rhs, _, ss, ok := cs.parseExpr(block, stmt.Rhs[0])
+			rhs, _, 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])
+			lhs, ts, ss, ok := cs.parseExpr(block, stmt.Lhs[0], false)
 			if !ok {
 				return nil, false
 			}
@@ -111,7 +111,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 			cs.addError(stmt.Pos(), fmt.Sprintf("unexpected token: %s", stmt.Tok))
 		}
 	case *ast.BlockStmt:
-		b, ok := cs.parseBlock(block, fname, stmt.List, inParams, outParams)
+		b, ok := cs.parseBlock(block, fname, stmt.List, inParams, outParams, true)
 		if !ok {
 			return nil, false
 		}
@@ -146,7 +146,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		// Create a new pseudo block for the initial statement, so that the counter variable belongs to the
 		// new pseudo block for each for-loop. Without this, the samely named counter variables in different
 		// for-loops confuses the parser.
-		pseudoBlock, ok := cs.parseBlock(block, fname, []ast.Stmt{stmt.Init}, inParams, outParams)
+		pseudoBlock, ok := cs.parseBlock(block, fname, []ast.Stmt{stmt.Init}, inParams, outParams, false)
 		if !ok {
 			return nil, false
 		}
@@ -173,7 +173,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		vartype := pseudoBlock.vars[0].typ
 		init := ss[0].Exprs[1].Const
 
-		exprs, ts, ss, ok := cs.parseExpr(pseudoBlock, stmt.Cond)
+		exprs, ts, ss, ok := cs.parseExpr(pseudoBlock, stmt.Cond, true)
 		if !ok {
 			return nil, false
 		}
@@ -258,7 +258,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 			return nil, false
 		}
 
-		b, ok := cs.parseBlock(pseudoBlock, fname, []ast.Stmt{stmt.Body}, inParams, outParams)
+		b, ok := cs.parseBlock(pseudoBlock, fname, []ast.Stmt{stmt.Body}, inParams, outParams, true)
 		if !ok {
 			return nil, false
 		}
@@ -289,7 +289,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		if stmt.Init != nil {
 			init := stmt.Init
 			stmt.Init = nil
-			b, ok := cs.parseBlock(block, fname, []ast.Stmt{init, stmt}, inParams, outParams)
+			b, ok := cs.parseBlock(block, fname, []ast.Stmt{init, stmt}, inParams, outParams, true)
 			if !ok {
 				return nil, false
 			}
@@ -301,7 +301,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 			return stmts, true
 		}
 
-		exprs, ts, ss, ok := cs.parseExpr(block, stmt.Cond)
+		exprs, ts, ss, ok := cs.parseExpr(block, stmt.Cond, true)
 		if !ok {
 			return nil, false
 		}
@@ -316,7 +316,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		stmts = append(stmts, ss...)
 
 		var bs []*shaderir.Block
-		b, ok := cs.parseBlock(block, fname, stmt.Body.List, inParams, outParams)
+		b, ok := cs.parseBlock(block, fname, stmt.Body.List, inParams, outParams, true)
 		if !ok {
 			return nil, false
 		}
@@ -325,13 +325,13 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		if stmt.Else != nil {
 			switch s := stmt.Else.(type) {
 			case *ast.BlockStmt:
-				b, ok := cs.parseBlock(block, fname, s.List, inParams, outParams)
+				b, ok := cs.parseBlock(block, fname, s.List, inParams, outParams, true)
 				if !ok {
 					return nil, false
 				}
 				bs = append(bs, b.ir)
 			default:
-				b, ok := cs.parseBlock(block, fname, []ast.Stmt{s}, inParams, outParams)
+				b, ok := cs.parseBlock(block, fname, []ast.Stmt{s}, inParams, outParams, true)
 				if !ok {
 					return nil, false
 				}
@@ -346,7 +346,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		})
 
 	case *ast.IncDecStmt:
-		exprs, _, ss, ok := cs.parseExpr(block, stmt.X)
+		exprs, _, ss, ok := cs.parseExpr(block, stmt.X, false)
 		if !ok {
 			return nil, false
 		}
@@ -388,7 +388,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		}
 
 		for i, r := range stmt.Results {
-			exprs, ts, ss, ok := cs.parseExpr(block, r)
+			exprs, ts, ss, ok := cs.parseExpr(block, r, true)
 			if !ok {
 				return nil, false
 			}
@@ -463,7 +463,7 @@ func (cs *compileState) parseStmt(block *block, fname string, stmt ast.Stmt, inP
 		}
 
 	case *ast.ExprStmt:
-		exprs, _, ss, ok := cs.parseExpr(block, stmt.X)
+		exprs, _, ss, ok := cs.parseExpr(block, stmt.X, true)
 		if !ok {
 			return nil, false
 		}
@@ -495,7 +495,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])
+			r, origts, ss, ok := cs.parseExpr(block, rhs[i], true)
 			if !ok {
 				return nil, false
 			}
@@ -520,13 +520,11 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r
 					return nil, false
 				}
 
-				v := variable{
+				var t shaderir.Type
-					name: name,
-				}
 				if len(ts) == 1 {
-					v.typ = ts[0]
+					t = ts[0]
 				}
-				block.vars = append(block.vars, v)
+				block.addNamedLocalVariable(name, t, e.Pos())
 			}
 
 			if len(r) > 1 {
@@ -534,7 +532,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])
+			l, _, ss, ok := cs.parseExpr(block, lhs[i], false)
 			if !ok {
 				return nil, false
 			}
@@ -618,7 +616,7 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r
 			if i == 0 {
 				var ss []shaderir.Stmt
 				var ok bool
-				rhsExprs, rhsTypes, ss, ok = cs.parseExpr(block, rhs[0])
+				rhsExprs, rhsTypes, ss, ok = cs.parseExpr(block, rhs[0], true)
 				if !ok {
 					return nil, false
 				}
@@ -638,14 +636,10 @@ func (cs *compileState) assign(block *block, fname string, pos token.Pos, lhs, r
 						}
 					}
 				}
-				v := variable{
+				block.addNamedLocalVariable(name, rhsTypes[i], e.Pos())
-					name: name,
-				}
-				v.typ = rhsTypes[i]
-				block.vars = append(block.vars, v)
 			}
 
-			l, _, ss, ok := cs.parseExpr(block, lhs[i])
+			l, _, ss, ok := cs.parseExpr(block, lhs[i], false)
 			if !ok {
 				return nil, false
 			}

internal/shader/testdata/blank.go

@@ -8,4 +8,5 @@ func Bar() {
 	_, _ = Foo()
 	a, _ := Foo()
 	_, b := Foo()
+	_, _ = a, b
 }

internal/shader/testdata/blocks2.go

@@ -6,5 +6,6 @@ func Foo() vec2 {
 		x := 0
 		return vec2(x)
 	}
+	_ = x
 	return vec2(1)
 }

internal/shader/testdata/define2.go

@@ -3,6 +3,7 @@ package main
 func Foo() vec2 {
 	x0 := 1 * Bar()
 	x1 := Bar() * 1
+	_ = x1
 	return x0
 }
 

internal/shader/testdata/define_multiple.go

@@ -2,6 +2,7 @@ package main
 
 func Foo(foo vec2) vec4 {
 	r1, r2 := Bar()
+	_ = r2
 	return vec4(foo, r1, r1)
 }
 

internal/shader/testdata/for.go

@@ -9,5 +9,6 @@ func Foo() vec2 {
 	for i := 10.0; i >= 0; i -= 2 {
 		v2.x += i
 	}
+	_ = v2
 	return v
 }

internal/shader/testdata/for3.go

@@ -11,5 +11,6 @@ func Foo() vec2 {
 		v4 := vec2(0)
 		v3 = v4
 	}
+	_ = v3
 	return v
 }

internal/shader/type.go

@@ -57,7 +57,7 @@ func (cs *compileState) parseType(block *block, expr ast.Expr) (shaderir.Type, b
 		if _, ok := t.Len.(*ast.Ellipsis); ok {
 			length = -1 // Determine the length later.
 		} else {
-			exprs, _, _, ok := cs.parseExpr(block, t.Len)
+			exprs, _, _, ok := cs.parseExpr(block, t.Len, true)
 			if !ok {
 				return shaderir.Type{}, false
 			}

shader_test.go

@@ -518,3 +518,15 @@ func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
 		t.Errorf("error must be nil but was non-nil")
 	}
 }
+
+func TestShaderUnusedVariable(t *testing.T) {
+	if _, err := NewShader([]byte(`package main
+
+func Fragment(position vec4, texCoord vec2, color vec4) vec4 {
+	x := 0
+	return vec4(0)
+}
+`)); err == nil {
+		t.Errorf("error must be non-nil but was nil")
+	}
+}