Opened 3 years ago

Closed 13 months ago

#9022 closed bug (fixed)

TH pretty printer and GHC parser semicolon placement mismatch

Reported by: roldugin Owned by: bollmann
Priority: normal Milestone:
Component: Template Haskell Version: 7.8.1
Keywords: newcomer Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1898
Wiki Page:


In GHC 7.8 TemplateHaskell pretty printer started inserting explicit braces and semicolons.

It puts semicolons at the end of the line as opposed to the beginning of the next line.

This causes GHC to fail parsing if we try to compile the pretty printed code.

$ cat Foo.hs
module Main where

import Language.Haskell.TH

main = putStrLn $ pprint foo

foo :: Dec
foo = barD
       barD = FunD ( mkName "bar" )
                   [ Clause manyArgs (NormalB barBody) [] ]

       barBody = DoE [letxStmt, retxStmt]

       letxStmt = LetS [ ValD (VarP xName) (NormalB $ LitE $ IntegerL 5) [] ]

       retxStmt = NoBindS $ AppE returnVarE xVarE

       xName = mkName "x"

       returnVarE = VarE $ mkName "return"

       xVarE = VarE xName

       manyArgs = map argP [0..9]

       argP n = VarP $ mkName $ "arg" ++ show n

$ ghc-7.8.2 Foo.hs
[1 of 1] Compiling Main             ( Foo.hs, Foo.o )
Linking Foo ...

$ ./Foo | tee Bar.hs
bar arg0 arg1 arg2 arg3 arg4 arg5 arg6 arg7 arg8 arg9 = do {let x = 5;
                                                            return x}

$ ghc Bar.hs
[1 of 1] Compiling Main             ( Bar.hs, Bar.o )

Bar.hs:2:61: parse error on input `return'

I don't know if this is a problem with TH pretty printer or if GHC is supposed to parse semicolons wherever they are..

Attachments (1)

9022.patch (1.5 KB) - added by roldugin 2 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 2 years ago by thomie

Component: Compiler (Parser)Test Suite
Milestone: 7.10.1

Thank you for the report.

I confirm this problem does not occur in 7.6.3, and does occur with 7.8.1 and HEAD. I can not find anything about this in the 7.8.1 changelog. I did find a related change, but then for the GHC Api pretty printer.

commit 6f32f9b429814499cfde1367e59d3e46bffc4925
Author: <unknown>
Date:   Thu Jul 23 15:24:11 2009 +0000

    Print explicit braces and semicolons in do-notation
    By printing explicit braces we make it more likely that pretty-printed
    code will be acceptable if fed back into GHC.

comment:2 Changed 2 years ago by thomie

Component: Test SuiteTemplate Haskell

comment:3 Changed 2 years ago by simonpj

I think the solution is not to be very delicate about how to place the semicolons for the do-notation. Rather, we should just use braces and semicolons for let-notation too. At least for lets in do-notation but perhaps for all.

Then you'd get

bar arg0 arg1 arg2 arg3 arg4 arg5 arg6 arg7 arg8 arg9 = do {let { x = 5 };
                                                            return x}

which should work fine.

Anyone want to do this? It would be consistent with the story for do-notation.


Changed 2 years ago by roldugin

Attachment: 9022.patch added

comment:4 Changed 2 years ago by roldugin

There was a discussion in the mailing list and I fixed it based on advice I got there.

The fix worked for me but I haven't validated it.

I no longer have write access to the repo, so I've attached it above.

comment:5 Changed 2 years ago by simonpj

Did you test your fix?

let {x = 3} ; {y = 4} in x+y

is not valid Haskell, but is (I believe) what your patch will generate.


comment:6 Changed 2 years ago by roldugin

You're right!

let expressions should be fine:

> pprint <$> runQ [| let { x = 3; y = 4 } in x + y |]
"let {x_0 = 3; y_1 = 4}\n in x_0 GHC.Num.+ y_1"

But there is indeed a problem with let statements:

> pprint <$> runQ [| do let { x = 3; y = 4 } ; return (x + y) |]
"do {let {x_0 = 3}; {y_1 = 4}; GHC.Base.return (x_0 GHC.Num.+ y_1)}"

I think pretty printing LetS will need to use custom pprDecs like LetE does around line 149.

Unfortunately, I can't do it right now but I will when I get a minute.


comment:7 Changed 2 years ago by goldfire

Keywords: newcomer added

comment:8 Changed 2 years ago by thoughtpolice


Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:9 Changed 2 years ago by thomie

Owner: set to roldugin

roldugin: assigning to you, since you were working on this.

comment:10 Changed 18 months ago by thoughtpolice


Milestone renamed

comment:11 Changed 18 months ago by thomie

Owner: roldugin deleted

comment:12 Changed 13 months ago by thomie

Milestone: 8.0.1

comment:13 Changed 13 months ago by bollmann

It seems that this issue was already fixed in ticket #10734. I just added the above test locally and it passes in the current head:

Shall I add the above testcase as part of the TH testsuite and then close the ticket? Or shall we just close this ticket right away?

comment:14 Changed 13 months ago by bollmann

Owner: set to bollmann

comment:15 Changed 13 months ago by bollmann

Differential Rev(s): D1898
Status: newpatch

I just added the regression test mentioned above to the testsuite and pushed the code for review. However, I actually think, we should just close this ticket without adding the test (as the issue has already been fixed in #10734 and that ticket also added these sorts of tests). Let me know what you think.

Last edited 13 months ago by bollmann (previous) (diff)

comment:16 Changed 13 months ago by bollmann

Differential Rev(s): D1898Phab:D1898

comment:17 Changed 13 months ago by Ben Gamari <ben@…>

In 2f9931e3/ghc:

add Template Haskell regression test for #9022.

The bug itself has already been fixed in #10734, so this
only adds another regression test (as given in the ticket).

Test Plan: ./validate

Reviewers: goldfire, austin, thomie, bgamari

Reviewed By: bgamari

Differential Revision:

GHC Trac Issues: #9022

comment:18 Changed 13 months ago by bollmann

Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.