#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: #7666 | -------------------------------------+------------------------------------- Changes (by thomie): * owner: => thomie Comment: I have patches. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: #7666, #8809 | -------------------------------------+------------------------------------- Changes (by thomie): * related: #1062, #1176, #7666 => #1062, #1176, #7666, #8809 Comment: Ok, maybe you want to comment on https://github.com/haskell/pretty/issues/1#issuecomment-78297997, where Trevor Elliott said: "I was hoping to extend the GHC pretty printer with the annotations, to get functionality like in the idris repl. Maybe extending their forked version with the annotations is the right path forward for that work :)" -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: Phab:D1130 #7666, #8809 | -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"2d1eae26f83bdff3bc5f5eb98101e4fd718afd62/ghc" 2d1eae26/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="2d1eae26f83bdff3bc5f5eb98101e4fd718afd62" Pretty: kill code that has been dead since 1997 (#10735) }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: Phab:D1130 #7666, #8809 | -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"6f6d082124b24bd8437f95d99a8fd8844a0f6cd8/ghc" 6f6d082/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="6f6d082124b24bd8437f95d99a8fd8844a0f6cd8" Pretty: Args of NilAbove/TextBeside/Nest/Union are always RDocs (#10735) Just following libraries/pretty. }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: Phab:D1130 #7666, #8809 | -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"85179b5821bb1010eede7cec43280c2cd7e59bd3/ghc" 85179b58/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="85179b5821bb1010eede7cec43280c2cd7e59bd3" Pretty: use replicate for spaces and multi_ch (#10735) Similar changes were made to pretty in commit 7575ab16430c876eaa1451b02465b6b103b3a519. }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: Phab:D1130 #7666, #8809 | -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"d7b053a25f17a02753780293bc1d417c5794e91f/ghc" d7b053a/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="d7b053a25f17a02753780293bc1d417c5794e91f" Pretty: reformat using style from libraries/pretty (#10735) This commit copies the code structure (what goes where), whitespace layout and comments from libraries/pretty/src/Text/PrettyPrint/HughesPJ.hs, with the intention to be able to later more easily compare the two files, and port bug fixes. I'm sorry this messes up git blame history, but there's no other way. }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: Phab:D1130 #7666, #8809 | -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"53484d3deff9fa9f8b3da01a4b375b4ea8a7ba05/ghc" 53484d3d/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="53484d3deff9fa9f8b3da01a4b375b4ea8a7ba05" Pretty: remove superfluous parenthesis (#10735) }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: patch Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: #7666, #8809 | Phab:D1130, Phab:D1132 -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"bcfae08c0be0fa8604e2025733dfae57e37c2083/ghc" bcfae08c/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="bcfae08c0be0fa8604e2025733dfae57e37c2083" Pretty: fix potential bad formatting of error message (#10735) This is a backport of a bug fix by Benedikt Huber for the same problem in the pretty library (#1337), from commit 8d8866a8379c2fe8108ef034893c59e06d5e752f. The original explanation for the fix is attached below. Ticket #1776 originally reported an infinite loop when printing error message. This promptly got fixed in: commit 2d52ee06786e5caf0c2d65a4b4bb7c45c6493190 Author: simonpj@microsoft.com <unknown> Date: Thu Mar 1 11:45:13 2007 +0000 Do not go into an infinite loop when pretty-printer finds a negative indent (Trac #1176) SPJ reports in the ticket: "So infinite loop is fixed, but the bad formatting remains. I've added a test, tcfail177." tcfail177 however hasn't triggered the formatting problem for years (as Ian reported in c9e0e6067a47c574d9ff3721afe58e30ca1be3e4). This patch updates the test to a version that at least still failed with ghc-7.0 (from #1776#comment:7). ------------------- From https://mail.haskell.org/pipermail/libraries/2008-June/010013.html, by Benedikt Huber: Concerning ticket #1337, we have to change the formal specification of fill (it doesn't match the implementation): -- Current Specification: -- fill [] = empty -- fill [p] = p -- fill (p1:p2:ps) = oneLiner p1 <#> nest (length p1) -- (fill (oneLiner p2 : ps)) -- `union` -- p1 $$ fill ps Problem 1: We want to `unnest' the second argument of (p1 $$ fill ps), but not the first one In the definition above we have e.g. > getSecondLayout $ > fillDef False [text "a", text "b", text "a"] >> text "ab"; nilabove; nest -1; text "a"; empty >> |ab| >> |.a| Problem 2: The overlapping $$ should only be used for those layouts of p1 which aren't one liners (otherwise violating the invariant "Left union arg has shorter first line"). I suggest the following specification (i believe it almost matches the current implementation, modulo [fillNB: fix bug #1337] (see below): -- Revised Specification: -- fill g docs = fill' 0 docs -- gap g = if g then 1 else 0 -- fill' n [] = [] -- fill' n [p] = [p] -- fill' n (p1:p2:ps) = -- oneLiner p1 <g> (fill' (n+length p1+gap g) (oneLiner p2 : ps)) -- `union` -- (p1 $*$ nest (-n) (fill' g ps)) -- -- $*$ is defined for layouts (One-Layout Documents) as -- -- layout1 $*$ layout2 | isOneLiner layout1 = layout1 $+$ layout2 -- | otherwise = layout1 $$ layout2 I've also implemented the specification in HughesPJQuickCheck.hs, and checked them against the patched pretty printer. Concerning Bug #1337: ~~~~~~~~~~~~~~~~~~~~~ If the above formal specification is fine, it is easy to fix: elide the nests of (oneLiner p2) [see attached patch, record bug #1337]. > PrettyPrint(0) $ ./Bug1337 > ....ab > ...c The (long) explanation follows below. <snip/> =========================================================== Explanation of Bug #1337: Consider > fcat [ nest 1 $ text "a", nest 2 $ text "b", text "c"] --> expected: (nest 1; text "a"; text "b"; nest -3; "c") --> actual : (nest 1; text "a"; text "b"; nest -5; "c") Reduction: === (nest 1; text a) <> (fill (-2) (p2:ps)) ==> (nest 2 (text "b") $+$ text "c") ==> (nest 2 (text "b")) `nilabove` (nest (-3) (text "c")) ==> (nest 1; text a; text b; nest -5 c) The problem is that if we decide to layout (p1:p2:ps) as | p1 p2 | ps (call it layout A), then we want to have > (p1 <> p2) $+$ ps. But following law <n6> this means that > fcat_A [p1:nest k p2:ps] is equivalent to > fcat_A [p1,p2,ps] so the nest of p2 has to be removed. This is somewhat similar to bug #667, but easier to fix from a semantic point of view: p1,p2 and ps are distinct layouts - we only have to preserve the individual layouts, and no combinations of them. }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: patch Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: #7666, #8809 | Phab:D1130, Phab:D1132 -------------------------------------+------------------------------------- Comment (by Thomas Miedema ): In [changeset:"5d57087e314bd484dbe14958f9b422be3ac6641a/ghc" 5d57087/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="5d57087e314bd484dbe14958f9b422be3ac6641a" Pretty: fix a broken invariant (#10735) This is a backport of a bug fix from 6cfbd0444981c074bae10a3cf72733bcb8597bef in libraries/pretty: Fix a broken invariant Patch from #694, for the problem "empty is an identity for <> and $$" is currently broken by eg. isEmpty (empty<>empty)" }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: patch Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Revisions: #7666, #8809 | Phab:D1130, Phab:D1132 -------------------------------------+------------------------------------- Comment (by thomie): There is 1 commit left for parity with pretty-1.1.2.0. I have not landed it yet, because I haven't been able to measure the supposed space/time improvement yet. The other 2 commits described below seem to suggest this commit doesn't actually work: "Pretty: improving the space/time performance of vcat, hsep, hcat" There are 2 commits left for parity with pretty-1.1.2.1. I have not landed them yet, because they seem to cause more allocation in the compiler (see discussion in https://github.com/haskell/pretty/pull/9): "Resolve foldr-strictness stack overflow bug" "Special-case reduce for horiz/vert" The commits are here: https://github.com/thomie/ghc/commits/pretty -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: thomie Type: task | Status: patch Priority: normal | Milestone: 8.2.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Rev(s): Phab:D1130, #7666, #8809 | Phab:D1132 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.0.1 => 8.2.1 Comment: It seems unlikely that this is going to happen for 8.0. Punting to 8.2. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:22 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: Type: task | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Rev(s): Phab:D1130, #7666, #8809 | Phab:D1132 Wiki Page: | -------------------------------------+------------------------------------- Changes (by simonpj): * status: closed => new * owner: thomie => * resolution: fixed => Comment: Re-opening, because we'd really like to start using the real `pretty` library for GHC. It just needs someone to finish the work Thomas started! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:24 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10735: Smooth out the differences between `compiler/utils/Pretty.hs` and `libraries/pretty` -------------------------------------+------------------------------------- Reporter: thomie | Owner: Type: task | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: #1062, #1176, | Differential Rev(s): Phab:D1130, #7666, #8809 | Phab:D1132 Wiki Page: | -------------------------------------+------------------------------------- Changes (by michalt): * cc: michalt (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10735#comment:26 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler