Fix pretty-printing of infix operators

[byorgey]

Infix operators do not print correctly at the moment; they print as if they were prefix operations, like + 2 3.

Fixing this will be critical to being able to round-trip pretty-printing and parsing, which in turn is needed to be able to support #7 .

[xsebek]

Hi, I took a look at what this would look like and from what I found:

• operators are made by mkOp as TApp of some TConst:

  -- | Make infix operation (e.g. @2 + 3@) a curried function
  --   application (@((+) 2) 3@).
  --
  -- >>> mkOp (Arith Add) (TInt 2) (TInt 3)
  -- TApp (TApp (TConst (Arith Add)) (TInt 2)) (TInt 3)
  mkOp :: Const -> Term -> Term -> Term
  mkOp c = TApp . TApp (TConst c)

• correct printing requires adjusting PrettyPrec Term - something like:

  -- Special handling of infix operators - ((+) 2) 3 --> 2 + 3
  prettyPrec p (TApp (TApp (TConst c) l) r) = hsep
    [prettyPrec p l, ppr c, prettyPrec p r]
  prettyPrec p (TApp t1 t2)  = pparens (p > 10) $
    prettyPrec 10 t1 <+> prettyPrec 11 t2

Can any other TConst apart from Arith and Cmp be applied? And do I have to respect the precedence here?

[byorgey]

Hi, thanks for taking a look at this! You're absolutely right that the pretty printer would need a special case for nested TApps of a TConst. But yes, any TConst that takes arguments (i.e. with arity > 0) will also be applied via TApp. So given a TApp (TApp (TConst c) l) r we also need to check if the constant c is infix. And yes, precedence will come into play as well.

I think I see three levels for a solution here, ranging from the simplest thing that will work and enable #7 , up to the one that will be the most work but will be the best long-term solution. It is totally up to you what approach you want to take here --- e.g. you could just implement the simplest fix for now and spin off other issue(s) to tackle later, or you could just go straight for the more principled approach that is more work.

Level 1

We can add a function isInfix : Const -> Bool to Swarm.Language.Syntax. Fortunately this function is very easy to write: Arith and Cmp things are infix, and nothing else. Then to avoid having to worry about precedence, we can simply pretty-print every infix constant application with parentheses around it. e.g. 1 + 2 * 3 would be pretty-printed as (1 + (2 * 3)). This is a bit ugly, of course, but it is easy and correct.

Level 2

To correctly handle precedence, we need to make another function prec : Const -> Int which returns the precedence level of each infix constant (it can just return anything, say 0, for the others). To know what the proper precedence levels are, take a look at parseExpr in the parser, or just use the same precedence levels that Haskell uses. Then you can use the mparens function to decide whether to print parentheses or not (see some examples of how it is used in the rest of the pretty-printer; I'm happy to provide more explicit guidance for anyone who is not sure how this works).

Level 3

A bigger problem is that now a lot of information about constants is fragmented around the codebase: the arity function, isInfix function, and isCmd function encode data about each constant; the precedence of infix constants is now encoded in two separate places (implicitly in the parser, and explicitly in the prec function); and also the information about what concrete textual symbol to use for each constant is also encoded in two separate places (the parser and pretty-printer). It would be too easy for these to get out of sync, and when adding a new constant it is difficult to know all the places that must be updated.

A much better way is to define a data type to represent all the information about a constant, and a single function to output this information for each constant, something like

data ConstInfo = ConstInfo
  { syntax :: Text
  , arity :: Int
  , isCmd :: Bool
  , isInfix :: Bool
  , precedence :: Int
  }

constInfo :: Const -> ConstInfo
constInfo = \case
  Wait -> ConstInfo "wait" 0 True False 0
  ...
  Arith Add -> ConstInfo "+" 2 False True 6
  ...

This will replace the arity, isInfix, isCmd, and prec functions. We should probably get rid of the separate ArithConst and CmpConst types and just inline all into the single Const type; that way, we can derive Enum and Bounded for Const and get a list of all constants by [minBound .. maxBound]. Then the parser and pretty-printer no longer need to give an explicit list of all the constants; we can simply derive the relevant parts of the parser and pretty-printer from the list of all constants and the constInfo function. The ultimate goal is that in order to add a new constant to the language, we only have to add a constructor to the Const type, and a case to the constInfo function, and that's it! The parser and pretty-printer don't need to be touched since they will automatcially pick up the new constant and use the relevant info to do their job.

[byorgey]

Hi @xsebek , are you thinking of working on this (or are you already)? I'm kind of itching to implement the "Level 3" thing in my comment above, because I'm getting very annoyed having to edit like 7 or 8 files every time I add a single command. But if you're already working on any of this I don't want to step on your toes, and there are plenty of other things I can work on.