[GHC] #15815: problem with splicing type into constraint

[GHC]

#15815: problem with splicing type into constraint
-------------------------------------+-------------------------------------
        Reporter:  int-e             |                Owner:  RyanGlScott
            Type:  bug               |               Status:  new
        Priority:  highest           |            Milestone:
       Component:  Template Haskell  |              Version:  8.6.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
 Type of failure:  GHC rejects       |  Unknown/Multiple
  valid program                      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #15760            |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------
Changes (by int-index):

 * cc: int-index (added)


Comment:

 Ryan, why do you believe that preserving parentheses would solve this
 issue? The original code does not have any parentheses (unlike your
 minimised version).

 The issue here appears to be manyfold

 1. We reassociate type operators according to their fixities, so with `a ~
 $(tyQ)` and `[t| Int -> Int |]` from the original report we get `a ~ Int
 -> Int` (no parens!) which gets correctly associated as `(a ~ Int) ->
 Int`. That's not the behaviour I would expect, but I've just learned that
 it's documented in `Note [Converting UInfix]`.
 2. However, we map all three of `(~) a b`, `a ~ b`, and `(a ~ b)` to `AppT
 (AppT EqualityT a) b`. So we discard both parenthesization information (as
 you noted) and whether equality was applied in an infix or a prefix
 fashion.
 3. When we convert back to `HsType`, we use `HsOpTy` if there are two
 operands, even if `(~)` was prefix in the original code. This means that
 even if we fix #15760, we will still get incorrect behaviour in the `(~) a
 b` case.

 For expressions, we have a distinction between `UInfixE` and `InfixE`:

 * `UInfixE` gets reassociated as necessary (documented in `See Note
 [Operator association]`)
 * `InfixE` gets parenthesised as necessary (documented in `Note
 [Converting UInfix]`)

 In types we have a similar distinction – `UInfixT` and `InfixT`. The
 former must get reassociated, and the latter parenthesised.

 NB. Looking at the code that handles `InfixT` it appears broken to me
 (because unlike code for expressions, it does not call
 `parenthesizeHsType`). But this is either a bug or I'm missing something.
 For the sake of the argument let's assume `InfixT` is treated the same way
 as `InfixE` – by parenthesising arguments as necessary.

 The question is: do we treat `AppT (AppT (EqualityT a)) b` as a moral
 equivalent of `UInfixT` or `InfixT`? The bug seems to be that we used to
 treat it as `InfixT`, now we treat it as `UInfixT`. Your patch with adding
 `parenthesizeHsType` seems to make it treated like `InfixT` again, but
 ideally the fix should be

 * to stop confusing `(~) a b` and `a ~ b`: map the former to `AppT (AppT
 (EqualityT a)) b` and the latter to `InfixT a EqualityT b`
 * to fix the treatment of `InfixT` in `cvtTypeKind` – parenthesise as
 necessary in expressions
 * drop this silly special case:
   {{{
              | [x',y'] <- tys' ->
                    returnL (HsOpTy noExt x' (noLoc eqTyCon_RDR) y')
   }}}

-- 
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/15815#comment:5>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler