[GHC] #14875: -ddump-splices pretty-printing oddities with case statements
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: Debugging Unknown/Multiple | information is incorrect Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- The latest installment in "Ryan finds minor bugs in `-ddump-splices`". Take this program: {{{#!hs {-# LANGUAGE ScopedTypeVariables #-} {-# LANGUAGE TemplateHaskell #-} {-# OPTIONS_GHC -ddump-splices #-} module Bug where $([d| f :: Bool -> Bool f x = case x of (True :: Bool) -> True (False :: Bool) -> False g :: Bool -> Bool g x = (case x of True -> True False -> False) :: Bool |]) }}} Compiling this gives: {{{ $ /opt/ghc/8.2.2/bin/ghci Bug.hs GHCi, version 8.2.2: http://www.haskell.org/ghc/ :? for help Loaded GHCi configuration from /home/rgscott/.ghci [1 of 1] Compiling Bug ( Bug.hs, interpreted ) Bug.hs:(6,3)-(15,6): Splicing declarations [d| f_a1sB :: Bool -> Bool f_a1sB x_a1sD = case x_a1sD of (True :: Bool) -> True (False :: Bool) -> False g_a1sC :: Bool -> Bool g_a1sC x_a1sE = (case x_a1sE of True -> True False -> False) :: Bool |] ======> f_a49Z :: Bool -> Bool f_a49Z x_a4a0 = case x_a4a0 of True :: Bool -> True False :: Bool -> False g_a49Y :: Bool -> Bool g_a49Y x_a4a1 = case x_a4a1 of True -> True False -> False :: Bool }}} Neither the `-ddump-splices` output for `f` nor `g` parse are legal Haskell: * In `f`, GHC fails to parenthesize the pattern signatures `True :: Bool` and `False :: Bool`. * In `g`, GHC fails to parenthesize the `case` expression which has an explicit `Bool` signature. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by alanz): * cc: alanz (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): I talked with alanz about this, and unfortunately, this isn't quite as straightforward to fix as with previous pretty-priner bugs. This is that we need to teach `cvtl` to put parentheses around expressions when converting `SigE`s. At the same time, we don't want to add //unnecessary// parentheses—we wouldn't want to convert `Just True :: Maybe Bool` to `(Just True) :: Maybe Bool`, for instance. Unfortunately, the machinery in GHC just isn't quite up to the task. Currently, we have the [http://git.haskell.org/ghc.git/blob/5819ae2173d4b16f1fde067d39c3c215a6adfe97... hsExprNeedsParens function], but this seems to assume that the argument `HsExpr` is occurring in a function application context. In this scenario, however, that's not the case: we have a `case` expression appearing in a type annotation context. Alas, `hsExprNeedsParens` cannot distinguish between the two contexts. I think the right path forward here is to introduce a new precedence argument to `hsExprNeedsParens`. Something like: {{{#!hs data Prec = TopPrec -- Top-level | SigPrec -- Argument of a type annotation (_ :: Foo) | OpPrec -- Argument of an infix operator (_ + 1) | AppPrec -- Argument of a prefix function (f _) }}} And use that to inform `hsExprNeedsParen` in the relevant cases. It's worth noting that this data type is tantalizingly close the existing [http://git.haskell.org/ghc.git/blob/5819ae2173d4b16f1fde067d39c3c215a6adfe97... TyPrec] data type: {{{#!hs data TyPrec = TopPrec -- No parens | FunPrec -- Function args; no parens for tycon apps | TyOpPrec -- Infix operator | TyConPrec -- Tycon args; no parens for atomic }}} Save for the fact that `TyPrec` currently doesn't have a `SigPrec` constructor, and `TyPrec` has this funny business with `FunPrec`. It might be worth considering if my proposed `Prec` and `TyPrec` could be merged. Alas, I've run out of time, so this won't be happening today. One last note: it turns out there's similar problems in the Template Haskell pretty-printer as well: {{{#!hs {-# LANGUAGE ScopedTypeVariables #-} {-# LANGUAGE TemplateHaskell #-} module Bug2 where import Language.Haskell.TH main :: IO () main = putStrLn $([d| f :: Bool -> Bool f x = case x of (True :: Bool) -> True (False :: Bool) -> False g :: Bool -> Bool g x = (case x of True -> True False -> False) :: Bool |] >>= stringE . pprint) }}} {{{ f_0 :: GHC.Types.Bool -> GHC.Types.Bool f_0 x_1 = case x_1 of GHC.Types.True :: GHC.Types.Bool -> GHC.Types.True GHC.Types.False :: GHC.Types.Bool -> GHC.Types.False g_2 :: GHC.Types.Bool -> GHC.Types.Bool g_2 x_3 = case x_3 of GHC.Types.True -> GHC.Types.True GHC.Types.False -> GHC.Types.False :: GHC.Types.Bool }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj): Yes, using a precedence argument must be the Right Thing. We do this in quite a few places, notably `TyCoRep` and even `RtClosureInspect` and `Debugger`. Maybe the thing to do is to have {{{ newtype PprPrec = Prec Int deriving( Eq, Ord ) }}} in `BasicTypes` or `Outputable`, and use it for everything. Then we can have local defns like {{{ topPrec = 0 funPrec = 1 ... }}} and the various applications of it won't conflict with each other. Maybe `topPrec` can be shared. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): Indeed. As currently designed, however, `TyPrec` doesn't fit neatly into this `PprPrec` paradigm, since the `Eq`/`Ord` instances for `TyPrec` treat `FunPrec` and `TyOpPrec` as having the same precedence (but they still need to be separate constructors, so sayeth [http://git.haskell.org/ghc.git/blob/cab3e6bfa8486c6c8eecac269c54d662f1371a0c... this note]). But in any case, I don't think I need to touch anything `Type`-related in this patch to begin with. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by simonpj):
As currently designed, however, TyPrec doesn't fit neatly into this PprPrec paradigm
Yes it does! The Note says that currently `FunPrec` and `TyOpPrec` are treated as equal. So we could say {{{ funPrec = Prec 2 tyOpPrec = Prec 2 }}} We have two names so that we can later give them different precedence if we want. As the note says {{{ But the two are different constructors of TyPrec so we could make (->) bind more or less tightly if we wanted. }}} Seems a neat fit to me. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): Oh, right you are. I thought that there was code in GHC that was //distinguishing// `FunPrec` from `TyOpPrec`, but after a quick audit, that does not appear to be the case. (Which leaves me a bit baffled that we didn't just define `funPrec = TyOpPrec` and avoided the need for such bizarre `Eq`/`Ord` instances. Oh well.) In that case, this should present a nice opportunity for some code cleanup. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: patch Priority: normal | Milestone: Component: Compiler | Version: 8.2.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4688 Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * status: new => patch * differential: => Phab:D4688 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: closed Priority: normal | Milestone: 8.6.1 Component: Compiler | Version: 8.2.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Debugging | Unknown/Multiple information is incorrect | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D4688 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: patch => closed * resolution: => fixed * milestone: => 8.6.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14875#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#14875: -ddump-splices pretty-printing oddities with case statements
-------------------------------------+-------------------------------------
Reporter: RyanGlScott | Owner: (none)
Type: bug | Status: closed
Priority: normal | Milestone: 8.6.1
Component: Compiler | Version: 8.2.2
Resolution: fixed | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Debugging | Unknown/Multiple
information is incorrect | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D4688
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
participants (1)
-
GHC