Follow up to #232. Operators built with lift2Decorated or with a positional MkBinaryOp (as the typed API does) set commutativity with a bare Bool, as in lift2Decorated (^) "pow" (Just "^") False 8 (Functions.hs#L463) and MkBinaryOp (==) "eq" (Just "==") True 4 (Typed/Expr.hs#L303). Nothing in the definition says what the Bool means, and #232 fixed two definitions where it was wrong.
Something like this would make each definition readable on its own:
data Commutativity = Commutative | NonCommutative
pow expr i = lift2Decorated (^) "pow" (Just "^") NonCommutative 8 expr (Lit i)
A wrong value is easy to miss because normalize (Expression.hs#L305-L313) swaps the operands of anything marked commutative before derive and filter evaluate it, so the result changes without any error. The aggregation planner (Plan.hs#L201) and the pretty printer (Expression.hs#L472) read the flag too.
Follow up to #232. Operators built with
lift2Decoratedor with a positionalMkBinaryOp(as the typed API does) set commutativity with a bareBool, as inlift2Decorated (^) "pow" (Just "^") False 8(Functions.hs#L463) andMkBinaryOp (==) "eq" (Just "==") True 4(Typed/Expr.hs#L303). Nothing in the definition says what theBoolmeans, and #232 fixed two definitions where it was wrong.Something like this would make each definition readable on its own:
A wrong value is easy to miss because
normalize(Expression.hs#L305-L313) swaps the operands of anything marked commutative beforederiveandfilterevaluate it, so the result changes without any error. The aggregation planner (Plan.hs#L201) and the pretty printer (Expression.hs#L472) read the flag too.