noteed / language-glsl

Haskell package for representation, parsing, and pretty-printing of GLSL
Other
31 stars 9 forks source link

Missing parentheses in prettyBinary #15

Closed conal closed 7 years ago

conal commented 7 years ago

The prettyBinary function in Language.GLSL.Pretty sometimes incorrectly omits parentheses. For instance, (/) is left-associative, so "a / b / c" parses as "(a / b) / c", i.e.,

Div (Div (Variable "a") (Variable "b")) (Variable "c")

and "a / (b / c)" parses as

Div (Variable "a") (Div (Variable "b") (Variable "c"))

However, pretty-printing this last expression yields "a / b / c", which is incorrect.

The problematic code:

prettyBinary :: Pretty a =>
  PrettyLevel -> Rational -> Rational -> String -> a -> a -> Doc
prettyBinary l p op o e1 e2 = prettyParen (p > op) $
  pPrintPrec l op e1 <+> text o <+> pPrintPrec l op e2

To fix this bug, tweak the precedence context when pretty-printing the argument expressions, depending on the operator's associative:

type Assoc = (Rational -> Rational, Rational -> Rational)

assocLeft, assocRight, assocNone :: Assoc
assocLeft  = (id,bump)
assocRight = (bump,id)
assocNone  = (bump,bump)

bump :: Rational -> Rational
bump = (+ 0.5)

prettyBinary :: Pretty a =>
  PrettyLevel -> Rational -> Rational -> Assoc -> String -> a -> a -> Doc
prettyBinary l p op (lf,rf) o e1 e2 = prettyParen (p > op) $
  pPrintPrec l (lf op) e1 <+> text o <+> pPrintPrec l (rf op) e2

I submitted a pull request.

noteed commented 7 years ago

Hi Conal,

Thanks a lot for the bug report and the PR! I've published the fixed package as version 0.2.1 to Hackage.

Thu

conal commented 7 years ago

Hi Thu. You're very welcome. Thanks for the quick merge and Hackage release, and thanks for language-glsl! I'm using it to help translate Haskell to GLSL, as part of my work on compiling to categories. This part of the project aims at reproducing Pan, Vertigo, and Eros in a much simpler way, with a nicer programming interface. It will combine categories for GLSL code generation, GUIs, and differentiable functions (for lighting).

noteed commented 7 years ago

Here's hoping you won't have too many problems with it; as you saw that library is quite rough and doesn't receive a lot of attention.

conal commented 7 years ago

@noteed I'm now using language-glsl-0.2.1 from Hackage in my project. Works fine. Thanks again.

schell commented 7 years ago

@conal is that project OSS? It may duplicate/replace my current work in that area so I'd like to follow along :)

conal commented 7 years ago

@schell Yes; the project I mentioned is OSS. See concat. The general scheme for non-standard compilation of Haskell programs is described in Compiling to Categories.

schell commented 7 years ago

Thanks @conal :)