zclsyntax: operations are now pointers to a struct value

Previously operations were an enum, but we've ended up needing to store
a collection of values against each, so an operation being a pointer to
a struct feels more natural.

This in turn allows us to more easily fix the return types of the
operations, so that we don't need to do any unusual work to understand
that (for example) arithmetic always returns a number.

zcl/zclsyntax/expression_ops.go

@@ -10,29 +10,78 @@ import (
 	"github.com/zclconf/go-zcl/zcl"
 )
 
-type Operation rune
+type Operation struct {
+	Impl function.Function
+	Type cty.Type
+}
 
-const (
+var (
-	OpNil Operation = 0 // Zero value of Operation. Not a valid Operation.
+	OpLogicalOr = &Operation{
+		Impl: stdlib.OrFunc,
+		Type: cty.Bool,
+	}
+	OpLogicalAnd = &Operation{
+		Impl: stdlib.AndFunc,
+		Type: cty.Bool,
+	}
+	OpLogicalNot = &Operation{
+		Impl: stdlib.NotFunc,
+		Type: cty.Bool,
+	}
 
-	OpLogicalOr          Operation = '∨'
+	OpEqual = &Operation{
-	OpLogicalAnd         Operation = '∧'
+		Impl: stdlib.EqualFunc,
-	OpLogicalNot         Operation = '!'
+		Type: cty.Bool,
-	OpEqual              Operation = '='
+	}
-	OpNotEqual           Operation = '≠'
+	OpNotEqual = &Operation{
-	OpGreaterThan        Operation = '>'
+		Impl: stdlib.NotEqualFunc,
-	OpGreaterThanOrEqual Operation = '≥'
+		Type: cty.Bool,
-	OpLessThan           Operation = '<'
+	}
-	OpLessThanOrEqual    Operation = '≤'
+
-	OpAdd                Operation = '+'
+	OpGreaterThan = &Operation{
-	OpSubtract           Operation = '-'
+		Impl: stdlib.GreaterThanFunc,
-	OpMultiply           Operation = '*'
+		Type: cty.Bool,
-	OpDivide             Operation = '/'
+	}
-	OpModulo             Operation = '%'
+	OpGreaterThanOrEqual = &Operation{
-	OpNegate             Operation = '∓'
+		Impl: stdlib.GreaterThanOrEqualToFunc,
+		Type: cty.Bool,
+	}
+	OpLessThan = &Operation{
+		Impl: stdlib.LessThanFunc,
+		Type: cty.Bool,
+	}
+	OpLessThanOrEqual = &Operation{
+		Impl: stdlib.LessThanOrEqualToFunc,
+		Type: cty.Bool,
+	}
+
+	OpAdd = &Operation{
+		Impl: stdlib.AddFunc,
+		Type: cty.Number,
+	}
+	OpSubtract = &Operation{
+		Impl: stdlib.SubtractFunc,
+		Type: cty.Number,
+	}
+	OpMultiply = &Operation{
+		Impl: stdlib.MultiplyFunc,
+		Type: cty.Number,
+	}
+	OpDivide = &Operation{
+		Impl: stdlib.DivideFunc,
+		Type: cty.Number,
+	}
+	OpModulo = &Operation{
+		Impl: stdlib.ModuloFunc,
+		Type: cty.Number,
+	}
+	OpNegate = &Operation{
+		Impl: stdlib.NegateFunc,
+		Type: cty.Number,
+	}
 )
 
-var binaryOps []map[TokenType]Operation
+var binaryOps []map[TokenType]*Operation
 
 func init() {
 	// This operation table maps from the operator's token type
@@ -42,7 +91,7 @@ func init() {
 	// Binary operator groups are listed in order of precedence, with
 	// the *lowest* precedence first. Operators within the same group
 	// have left-to-right associativity.
-	binaryOps = []map[TokenType]Operation{
+	binaryOps = []map[TokenType]*Operation{
 		{
 			TokenOr: OpLogicalOr,
 		},
@@ -71,30 +120,9 @@ func init() {
 	}
 }
 
-var operationImpls = map[Operation]function.Function{
-	OpLogicalAnd: stdlib.AndFunc,
-	OpLogicalOr:  stdlib.OrFunc,
-	OpLogicalNot: stdlib.NotFunc,
-
-	OpEqual:    stdlib.EqualFunc,
-	OpNotEqual: stdlib.NotEqualFunc,
-
-	OpGreaterThan:        stdlib.GreaterThanFunc,
-	OpGreaterThanOrEqual: stdlib.GreaterThanOrEqualToFunc,
-	OpLessThan:           stdlib.LessThanFunc,
-	OpLessThanOrEqual:    stdlib.LessThanOrEqualToFunc,
-
-	OpAdd:      stdlib.AddFunc,
-	OpSubtract: stdlib.SubtractFunc,
-	OpMultiply: stdlib.MultiplyFunc,
-	OpDivide:   stdlib.DivideFunc,
-	OpModulo:   stdlib.ModuloFunc,
-	OpNegate:   stdlib.NegateFunc,
-}
-
 type BinaryOpExpr struct {
 	LHS Expression
-	Op  Operation
+	Op  *Operation
 	RHS Expression
 
 	SrcRange zcl.Range
@@ -106,7 +134,7 @@ func (e *BinaryOpExpr) walkChildNodes(w internalWalkFunc) {
 }
 
 func (e *BinaryOpExpr) Value(ctx *zcl.EvalContext) (cty.Value, zcl.Diagnostics) {
-	impl := operationImpls[e.Op] // assumed to be a function taking exactly two arguments
+	impl := e.Op.Impl // assumed to be a function taking exactly two arguments
 	params := impl.Params()
 	lhsParam := params[0]
 	rhsParam := params[1]
@@ -142,16 +170,7 @@ func (e *BinaryOpExpr) Value(ctx *zcl.EvalContext) (cty.Value, zcl.Diagnostics)
 	if diags.HasErrors() {
 		// Don't actually try the call if we have errors already, since the
 		// this will probably just produce a confusing duplicative diagnostic.
-		// Instead, we'll use the function's type check to figure out what
+		return cty.UnknownVal(e.Op.Type), diags
-		// type would be returned, if possible.
-		args := []cty.Value{givenLHSVal, givenRHSVal}
-		retType, err := impl.ReturnTypeForValues(args)
-		if err != nil {
-			// can't even get a return type, so we'll bail here.
-			return cty.DynamicVal, diags
-		}
-
-		return cty.UnknownVal(retType), diags
 	}
 
 	args := []cty.Value{lhsVal, rhsVal}
@@ -165,11 +184,7 @@ func (e *BinaryOpExpr) Value(ctx *zcl.EvalContext) (cty.Value, zcl.Diagnostics)
 			Subject:  e.RHS.Range().Ptr(),
 			Context:  &e.SrcRange,
 		})
-		retType, err := impl.ReturnTypeForValues(args)
+		return cty.UnknownVal(e.Op.Type), diags
-		if err != nil {
-			return cty.DynamicVal, diags
-		}
-		return cty.UnknownVal(retType), diags
 	}
 
 	return result, diags
@@ -184,7 +199,7 @@ func (e *BinaryOpExpr) StartRange() zcl.Range {
 }
 
 type UnaryOpExpr struct {
-	Op  Operation
+	Op  *Operation
 	Val Expression
 
 	SrcRange    zcl.Range

zcl/zclsyntax/expression_test.go

@@ -36,11 +36,83 @@ func TestExpressionParseAndValue(t *testing.T) {
 			cty.NumberIntVal(5),
 			0,
 		},
+		{
+			`(2+unk)`,
+			&zcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"unk": cty.UnknownVal(cty.Number),
+				},
+			},
+			cty.UnknownVal(cty.Number),
+			0,
+		},
+		{
+			`(2+unk)`,
+			&zcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"unk": cty.DynamicVal,
+				},
+			},
+			cty.UnknownVal(cty.Number),
+			0,
+		},
+		{
+			`(unk+unk)`,
+			&zcl.EvalContext{
+				Variables: map[string]cty.Value{
+					"unk": cty.DynamicVal,
+				},
+			},
+			cty.UnknownVal(cty.Number),
+			0,
+		},
 		{
 			`(2+true)`,
 			nil,
-			cty.DynamicVal, // FIXME: should be cty.UnknownVal(cty.Number)
+			cty.UnknownVal(cty.Number),
-			1,
+			1, // unsuitable type for right operand
+		},
+		{
+			`(false+true)`,
+			nil,
+			cty.UnknownVal(cty.Number),
+			2, // unsuitable type for each operand
+		},
+		{
+			`(5 == 5)`,
+			nil,
+			cty.True,
+			0,
+		},
+		{
+			`(5 == 4)`,
+			nil,
+			cty.False,
+			0,
+		},
+		{
+			`(1 == true)`,
+			nil,
+			cty.False,
+			0,
+		},
+		{
+			`("true" == true)`,
+			nil,
+			cty.False,
+			0,
+		},
+		{
+			`(true == "true")`,
+			nil,
+			cty.False,
+			0,
+		},
+		{
+			`(true != "true")`,
+			nil,
+			cty.True,
+			0,
 		},
 		{
 			`(

zcl/zclsyntax/parser.go

@@ -400,7 +400,7 @@ func (p *parser) parseTernaryConditional() (Expression, zcl.Diagnostics) {
 // parseBinaryOps calls itself recursively to work through all of the
 // operator precedence groups, and then eventually calls parseExpressionTerm
 // for each operand.
-func (p *parser) parseBinaryOps(ops []map[TokenType]Operation) (Expression, zcl.Diagnostics) {
+func (p *parser) parseBinaryOps(ops []map[TokenType]*Operation) (Expression, zcl.Diagnostics) {
 	if len(ops) == 0 {
 		// We've run out of operators, so now we'll just try to parse a term.
 		return p.parseExpressionWithTraversals()
@@ -410,7 +410,7 @@ func (p *parser) parseBinaryOps(ops []map[TokenType]Operation) (Expression, zcl.
 	remaining := ops[1:]
 
 	var lhs, rhs Expression
-	operation := OpNil
+	var operation *Operation
 	var diags zcl.Diagnostics
 
 	// Parse a term that might be the first operand of a binary
@@ -432,14 +432,14 @@ func (p *parser) parseBinaryOps(ops []map[TokenType]Operation) (Expression, zcl.
 	// instead of iteratively parsing only the remaining operators.
 	for {
 		next := p.Peek()
-		var newOp Operation
+		var newOp *Operation
 		var ok bool
 		if newOp, ok = thisLevel[next.Type]; !ok {
 			break
 		}
 
 		// Are we extending an expression started on the previous iteration?
-		if operation != OpNil {
+		if operation != nil {
 			lhs = &BinaryOpExpr{
 				LHS: lhs,
 				Op:  operation,
@@ -459,7 +459,7 @@ func (p *parser) parseBinaryOps(ops []map[TokenType]Operation) (Expression, zcl.
 		}
 	}
 
-	if operation == OpNil {
+	if operation == nil {
 		return lhs, diags
 	}