Support only GHC 9.0 #357

Merged

tfausak merged 74 commits from main into master 2021-11-28 15:42:35 +01:00

Conversation 18 Commits 74 Files Changed 619

tfausak commented 2021-11-02 13:02:31 +01:00 (Migrated from github.com)

This fixes #352. It is an alternative to #356.

This pull request adds support for GHC 9.0 but drops support for all other versions of GHC. Since I am not really familiar with the Brittany codebase, it was challenging for me to deal with that and CPP at the same time.

These changes mostly work, but I'm having some trouble with spaces. Somehow final newlines went away, and spaces between arguments to constructors are mysteriously missing.

This fixes #352. It is an alternative to #356. This pull request adds support for GHC 9.0 but drops support for all other versions of GHC. Since I am not really familiar with the Brittany codebase, it was challenging for me to deal with that and CPP at the same time. These changes mostly work, but I'm having some trouble with spaces. Somehow final newlines went away, and spaces between arguments to constructors are mysteriously missing.

tfausak (Migrated from github.com) reviewed 2021-11-02 13:10:02 +01:00

.vscode/extensions.json

tfausak (Migrated from github.com) commented 2021-11-02 13:03:28 +01:00

Unfortunately this doesn't really work. And it definitely won't work (for a while) if I upgrade to GHC 9.2.

Unfortunately this doesn't really work. And it definitely won't work (for a while) if I upgrade to GHC 9.2.

src-literatetests/Main.hs

tfausak (Migrated from github.com) commented 2021-11-02 13:03:49 +01:00

I need to revert this change.

I need to revert this change.

src/Language/Haskell/Brittany/Internal.hs

tfausak (Migrated from github.com) commented 2021-11-02 13:04:41 +01:00

I'm not sure if this change makes sense. AnnEofPos went away.

I'm not sure if this change makes sense. `AnnEofPos` went away.

src/Language/Haskell/Brittany/Internal/LayouterBasics.hs

tfausak (Migrated from github.com) commented 2021-11-02 13:06:37 +01:00

This function returns a "bad" real source span when a "good" one isn't available. Perhaps that messes up sorting somehow? Note that regular source spans can't be compared.

https://hackage.haskell.org/package/ghc-exactprint-1.2.0/docs/Language-Haskell-GHC-ExactPrint-Utils.html#v:rs

This function returns a "bad" real source span when a "good" one isn't available. Perhaps that messes up sorting somehow? Note that regular source spans can't be compared. https://hackage.haskell.org/package/ghc-exactprint-1.2.0/docs/Language-Haskell-GHC-ExactPrint-Utils.html#v:rs

src/Language/Haskell/Brittany/Internal/Layouters/DataDecl.hs

tfausak (Migrated from github.com) commented 2021-11-02 13:06:58 +01:00

createBndrDoc :: [LHsTyVarBndr flag GhcPs] -> ToBriDocM BriDocNumbered

```suggestion createBndrDoc :: [LHsTyVarBndr flag GhcPs] -> ToBriDocM BriDocNumbered ```

src/Language/Haskell/Brittany/Internal/Layouters/DataDecl.hs

tfausak (Migrated from github.com) commented 2021-11-02 13:07:49 +01:00

createForallDoc :: [LHsTyVarBndr flag GhcPs] -> Maybe (ToBriDocM BriDocNumbered)

```suggestion createForallDoc :: [LHsTyVarBndr flag GhcPs] -> Maybe (ToBriDocM BriDocNumbered) ```

src/Language/Haskell/Brittany/Internal/Layouters/DataDecl.hs

tfausak (Migrated from github.com) commented 2021-11-02 13:07:40 +01:00

https://downloads.haskell.org/~ghc/9.0.1/docs/html/libraries/ghc-9.0.1/GHC-Hs-Type.html#v:hsScaledThing

https://downloads.haskell.org/~ghc/9.0.1/docs/html/libraries/ghc-9.0.1/GHC-Hs-Type.html#v:hsScaledThing

tfausak (Migrated from github.com) reviewed 2021-11-05 00:43:03 +01:00

src/Language/Haskell/Brittany/Internal/Layouters/DataDecl.hs

tfausak (Migrated from github.com) commented 2021-11-05 00:43:02 +01:00

I initially flagged this because I thought the test failures were for data declarations and thought this might be the cause. However I think this is both unrelated to the test failures and overall harmless.

I initially flagged this because I thought the test failures were for data declarations and thought this might be the cause. However I think this is both unrelated to the test failures and overall harmless.

tfausak commented 2021-11-05 00:50:44 +01:00 (Migrated from github.com)

There are two failing test cases. They're both for type class instances that contain data (or newtype) declarations. The problem is that they're not inserting spaces between constructor arguments.

-- instance-with-data-family-below-method
-- expected
data MyData = Test Int Int
-- actual
data MyData = TestIntInt

-- instance-with-newtype-family-and-deriving
-- expected
newtype Bar () = Baz ()
-- actual
newtype Bar () = Baz()

https://github.com/tfausak/brittany/runs/4111574000?check_suite_focus=true#step:12:661

I think layoutDataFamInstDecl is ultimately responsible for this. Removing the call to stripWhitespace changes the output, but it doesn't fix the constructor arguments.

There are two failing test cases. They're both for type class instances that contain data (or newtype) declarations. The problem is that they're not inserting spaces between constructor arguments. ``` hs -- instance-with-data-family-below-method -- expected data MyData = Test Int Int -- actual data MyData = TestIntInt -- instance-with-newtype-family-and-deriving -- expected newtype Bar () = Baz () -- actual newtype Bar () = Baz() ``` https://github.com/tfausak/brittany/runs/4111574000?check_suite_focus=true#step:12:661 I think `layoutDataFamInstDecl` is ultimately responsible for this. Removing the call to `stripWhitespace` changes the output, but it doesn't fix the constructor arguments.

tfausak commented 2021-11-05 01:02:12 +01:00 (Migrated from github.com)

So layoutDataFamInstDecl just hands things off to briDocByExactNoComment, which seemingly works fine in other contexts. And also there's not a lot of room for things to go wrong there, assuming that ghc-exactprint is working correctly. I'm going to work on checking that assumption.

Perhaps it's possible that this is a GHC bug? I know there are a few gnarly ones in GHC 9.0.1. Unfortunately there's not yet a GHC 9.0.2 to test against.

So `layoutDataFamInstDecl` just hands things off to `briDocByExactNoComment`, which seemingly works fine in other contexts. And also there's not a lot of room for things to go wrong there, assuming that `ghc-exactprint` is working correctly. I'm going to work on checking that assumption. Perhaps it's possible that this is a GHC bug? I know there are a few gnarly ones in GHC 9.0.1. Unfortunately there's not yet a GHC 9.0.2 to test against.

tfausak commented 2021-11-05 02:18:37 +01:00 (Migrated from github.com)

Well at least ghc-exactprint seems to be doing the right thing:

$ cat GH357.hs 
{-# language TypeFamilies #-}
instance C T where
    data X T = X A B C

$ cabal exec -- brittany --exactprint-only GH357.hs 
{-# language TypeFamilies #-}
instance C T where
    data X T = X A B C

$ cabal exec -- brittany GH357.hs 
{-# language TypeFamilies #-}
instance C T where
    data X T = XABC

Well at least `ghc-exactprint` seems to be doing the right thing: ``` sh $ cat GH357.hs {-# language TypeFamilies #-} instance C T where data X T = X A B C $ cabal exec -- brittany --exactprint-only GH357.hs {-# language TypeFamilies #-} instance C T where data X T = X A B C $ cabal exec -- brittany GH357.hs {-# language TypeFamilies #-} instance C T where data X T = XABC ```

tfausak commented 2021-11-05 02:24:21 +01:00 (Migrated from github.com)

It looks like something is going wrong when converting the AST to BriDoc. That probably sounds obvious, but I feel it at least narrows things down. This isn't a problem with outputting the BriDoc, it's a problem with generating the BriDoc in the first place.

$ cabal exec -- brittany --suppress-output --dump-bridoc-raw GH357.hs

Click to see output.

---- bridoc raw ----
BDLines []
---- bridoc raw ----
BDLines
  [ BDExternal
      AnnKey {abstract:RealSrcSpan} (CN "ClsInstD")
      fromList
        [ AnnKey {abstract:RealSrcSpan} (CN "ClsInstD")
        , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar")
        , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
        , AnnKey {abstract:RealSrcSpan} (CN "HsAppTy")
        , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar")
        , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
        ]
      False
      pack "{-# language TypeFamilies #-}\ninstance C T where"
  , BDEnsureIndent
      BrIndentRegular
      BDIndentLevelPop
        BDIndentLevelPushCur
          BDLines
            [ BDExternal
                AnnKey {abstract:RealSrcSpan} (CN "DataFamInstDecl")
                fromList
                  [ AnnKey {abstract:RealSrcSpan} (CN "DataFamInstDecl")
                  , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
                  , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar")
                  , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
                  , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
                  , AnnKey {abstract:RealSrcSpan} (CN "ConDeclH98")
                  , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar")
                  , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
                  , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar")
                  , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
                  , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar")
                  , AnnKey {abstract:RealSrcSpan} (CN "Unqual")
                  , AnnKey {abstract:RealSrcSpan} (CN "False")
                  , AnnKey {abstract:RealSrcSpan} (CN "[]")
                  ]
                False
                pack "data X T = XABC"
            ]
  ]

It looks like something is going wrong when converting the AST to `BriDoc`. That probably sounds obvious, but I feel it at least narrows things down. This isn't a problem with outputting the `BriDoc`, it's a problem with generating the `BriDoc` in the first place. ``` sh $ cabal exec -- brittany --suppress-output --dump-bridoc-raw GH357.hs ``` <details> <summary>Click to see output.</summary> ``` hs ---- bridoc raw ---- BDLines [] ---- bridoc raw ---- BDLines [ BDExternal AnnKey {abstract:RealSrcSpan} (CN "ClsInstD") fromList [ AnnKey {abstract:RealSrcSpan} (CN "ClsInstD") , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "HsAppTy") , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") ] False pack "{-# language TypeFamilies #-}\ninstance C T where" , BDEnsureIndent BrIndentRegular BDIndentLevelPop BDIndentLevelPushCur BDLines [ BDExternal AnnKey {abstract:RealSrcSpan} (CN "DataFamInstDecl") fromList [ AnnKey {abstract:RealSrcSpan} (CN "DataFamInstDecl") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "ConDeclH98") , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "HsTyVar") , AnnKey {abstract:RealSrcSpan} (CN "Unqual") , AnnKey {abstract:RealSrcSpan} (CN "False") , AnnKey {abstract:RealSrcSpan} (CN "[]") ] False pack "data X T = XABC" ] ] ``` </details>

tfausak commented 2021-11-05 16:02:57 +01:00 (Migrated from github.com)

This was implied, but to make it explicit: The latest version of Brittany pretty prints that file without issue.

$ brittany --version
brittany version 0.13.1.2
Copyright (C) 2016-2019 Lennart Spitzner
Copyright (C) 2019 PRODA LTD
There is NO WARRANTY, to the extent permitted by law.

$ brittany GH357.hs
{-# language TypeFamilies #-}
instance C T where
  data X T = X A B C

Click to see raw BriDoc output. It's the same except for the source spans.

---- bridoc raw ----
BDLines []
---- bridoc raw ----
BDLines
  [ BDExternal
      AnnKey {GH357.hs:(2,1)-(3,22)} (CN "ClsInstD")
      fromList
        [ AnnKey {GH357.hs:(2,1)-(3,22)} (CN "ClsInstD")
        , AnnKey {GH357.hs:2:10} (CN "HsTyVar")
        , AnnKey {GH357.hs:2:10} (CN "Unqual")
        , AnnKey {GH357.hs:2:10-12} (CN "HsAppTy")
        , AnnKey {GH357.hs:2:12} (CN "HsTyVar")
        , AnnKey {GH357.hs:2:12} (CN "Unqual")
        ]
      False
      pack "{-# language TypeFamilies #-}\ninstance C T where"
  , BDEnsureIndent
      BrIndentRegular
      BDIndentLevelPop
        BDIndentLevelPushCur
          BDLines
            [ BDExternal
                AnnKey {GH357.hs:3:5-22} (CN "DataFamInstDecl")
                fromList
                  [ AnnKey {GH357.hs:3:5-22} (CN "DataFamInstDecl")
                  , AnnKey {GH357.hs:3:10} (CN "Unqual")
                  , AnnKey {GH357.hs:3:12} (CN "HsTyVar")
                  , AnnKey {GH357.hs:3:12} (CN "Unqual")
                  , AnnKey {GH357.hs:3:16} (CN "Unqual")
                  , AnnKey {GH357.hs:3:16-22} (CN "ConDeclH98")
                  , AnnKey {GH357.hs:3:18} (CN "HsTyVar")
                  , AnnKey {GH357.hs:3:18} (CN "Unqual")
                  , AnnKey {GH357.hs:3:20} (CN "HsTyVar")
                  , AnnKey {GH357.hs:3:20} (CN "Unqual")
                  , AnnKey {GH357.hs:3:22} (CN "HsTyVar")
                  , AnnKey {GH357.hs:3:22} (CN "Unqual")
                  , AnnKey {<no location info>} (CN "False")
                  , AnnKey {<no location info>} (CN "[]")
                  ]
                False
                pack "data X T = X A B C"
            ]
  ]

This was implied, but to make it explicit: The latest version of Brittany pretty prints that file without issue. ``` sh $ brittany --version brittany version 0.13.1.2 Copyright (C) 2016-2019 Lennart Spitzner Copyright (C) 2019 PRODA LTD There is NO WARRANTY, to the extent permitted by law. $ brittany GH357.hs {-# language TypeFamilies #-} instance C T where data X T = X A B C ``` <details> <summary>Click to see raw BriDoc output. It's the same except for the source spans.</summary> ``` ---- bridoc raw ---- BDLines [] ---- bridoc raw ---- BDLines [ BDExternal AnnKey {GH357.hs:(2,1)-(3,22)} (CN "ClsInstD") fromList [ AnnKey {GH357.hs:(2,1)-(3,22)} (CN "ClsInstD") , AnnKey {GH357.hs:2:10} (CN "HsTyVar") , AnnKey {GH357.hs:2:10} (CN "Unqual") , AnnKey {GH357.hs:2:10-12} (CN "HsAppTy") , AnnKey {GH357.hs:2:12} (CN "HsTyVar") , AnnKey {GH357.hs:2:12} (CN "Unqual") ] False pack "{-# language TypeFamilies #-}\ninstance C T where" , BDEnsureIndent BrIndentRegular BDIndentLevelPop BDIndentLevelPushCur BDLines [ BDExternal AnnKey {GH357.hs:3:5-22} (CN "DataFamInstDecl") fromList [ AnnKey {GH357.hs:3:5-22} (CN "DataFamInstDecl") , AnnKey {GH357.hs:3:10} (CN "Unqual") , AnnKey {GH357.hs:3:12} (CN "HsTyVar") , AnnKey {GH357.hs:3:12} (CN "Unqual") , AnnKey {GH357.hs:3:16} (CN "Unqual") , AnnKey {GH357.hs:3:16-22} (CN "ConDeclH98") , AnnKey {GH357.hs:3:18} (CN "HsTyVar") , AnnKey {GH357.hs:3:18} (CN "Unqual") , AnnKey {GH357.hs:3:20} (CN "HsTyVar") , AnnKey {GH357.hs:3:20} (CN "Unqual") , AnnKey {GH357.hs:3:22} (CN "HsTyVar") , AnnKey {GH357.hs:3:22} (CN "Unqual") , AnnKey {<no location info>} (CN "False") , AnnKey {<no location info>} (CN "[]") ] False pack "data X T = X A B C" ] ] ``` </details>

tfausak commented 2021-11-06 13:53:55 +01:00 (Migrated from github.com)

Here's an even simpler failing test case:

data instance A B = C D

As it stands on this branch, Brittany removes the space between C and D. Adding some tracing to docExt suggests that this is indeed a problem with exactPrint:

>>> docExt AnnKey TestFakeFileName.hs:1:1-23 CN "DataFamInstD"
>>> input: data instance A B = C D
>>> input (debug): {TestFakeFileName.hs:1:1-23}
data instance A{tc} {TestFakeFileName.hs:1:17}
                    B{tc}
  = {TestFakeFileName.hs:1:21-23}
    forall. C{d} D{tc}
>>> output: "data instance A B = CD"

Here's an even simpler failing test case: ``` hs data instance A B = C D ``` As it stands on this branch, Brittany removes the space between `C` and `D`. Adding some tracing to `docExt` suggests that this is indeed a problem with [`exactPrint`](https://hackage.haskell.org/package/ghc-exactprint-0.6.4/docs/Language-Haskell-GHC-ExactPrint.html#v:exactPrint): ``` >>> docExt AnnKey TestFakeFileName.hs:1:1-23 CN "DataFamInstD" >>> input: data instance A B = C D >>> input (debug): {TestFakeFileName.hs:1:1-23} data instance A{tc} {TestFakeFileName.hs:1:17} B{tc} = {TestFakeFileName.hs:1:21-23} forall. C{d} D{tc} >>> output: "data instance A B = CD" ```

tfausak commented 2021-11-06 14:21:32 +01:00 (Migrated from github.com)

Hmm, this is puzzling. I wrote a script to parse a module with ghc-exactprint and print it back out with exactPrint. I expected the same output as from Brittany's test suite, but it worked fine.

Click to see script.

import qualified GHC.Utils.Error
import qualified GHC.Utils.Outputable
import qualified Language.Haskell.GHC.ExactPrint
import qualified Language.Haskell.GHC.ExactPrint.Utils

main :: IO ()
main = do
    parseResult <- Language.Haskell.GHC.ExactPrint.parseModule "TestFakeFileName.hs"
    (anns, parsedSource) <- either
        ( fail
        . GHC.Utils.Outputable.showSDocUnsafe
        . GHC.Utils.Outputable.sep
        . GHC.Utils.Error.pprErrMsgBagWithLoc
        )
        pure
        parseResult

    putStrLn ">>> showSDocUnsafe"
    putStrLn
        . GHC.Utils.Outputable.showSDocUnsafe
        $ GHC.Utils.Outputable.ppr parsedSource

    putStrLn ">>> showSDocDebug_"
    putStrLn
        . Language.Haskell.GHC.ExactPrint.Utils.showSDocDebug_
        $ GHC.Utils.Outputable.ppr parsedSource

    putStrLn ">>> exactPrint"
    putStr $ Language.Haskell.GHC.ExactPrint.exactPrint parsedSource anns

>>> showSDocUnsafe
data instance A B = C D
>>> showSDocDebug_
{TestFakeFileName.hs:1:1}
{TestFakeFileName.hs:1:1-23}
data instance A{tc} {TestFakeFileName.hs:1:17}
                    B{tc}
  = {TestFakeFileName.hs:1:21-23}
    forall. C{d} D{tc}
>>> exactPrint
data instance A B = C D

I thought maybe the annotations were different, but I printed those out too and they looked fine.

Hmm, this is puzzling. I wrote a script to parse a module with `ghc-exactprint` and print it back out with `exactPrint`. I expected the same output as from Brittany's test suite, but it worked fine. <details> <summary>Click to see script.</summary> ``` hs import qualified GHC.Utils.Error import qualified GHC.Utils.Outputable import qualified Language.Haskell.GHC.ExactPrint import qualified Language.Haskell.GHC.ExactPrint.Utils main :: IO () main = do parseResult <- Language.Haskell.GHC.ExactPrint.parseModule "TestFakeFileName.hs" (anns, parsedSource) <- either ( fail . GHC.Utils.Outputable.showSDocUnsafe . GHC.Utils.Outputable.sep . GHC.Utils.Error.pprErrMsgBagWithLoc ) pure parseResult putStrLn ">>> showSDocUnsafe" putStrLn . GHC.Utils.Outputable.showSDocUnsafe $ GHC.Utils.Outputable.ppr parsedSource putStrLn ">>> showSDocDebug_" putStrLn . Language.Haskell.GHC.ExactPrint.Utils.showSDocDebug_ $ GHC.Utils.Outputable.ppr parsedSource putStrLn ">>> exactPrint" putStr $ Language.Haskell.GHC.ExactPrint.exactPrint parsedSource anns ``` </details> ``` >>> showSDocUnsafe data instance A B = C D >>> showSDocDebug_ {TestFakeFileName.hs:1:1} {TestFakeFileName.hs:1:1-23} data instance A{tc} {TestFakeFileName.hs:1:17} B{tc} = {TestFakeFileName.hs:1:21-23} forall. C{d} D{tc} >>> exactPrint data instance A B = C D ``` I thought maybe the annotations were different, but I printed those out too and they looked fine.

tfausak commented 2021-11-06 14:23:51 +01:00 (Migrated from github.com)

Maybe eyeballing the annotations wasn't sufficient 😆

Changing anns to addAnnotationsForPretty [] x anns fixes this problem but breaks a bunch of other test cases. So it looks like this case is failing because the annotations are wrong for some reason.

Maybe eyeballing the annotations wasn't sufficient 😆 Changing `anns` to `addAnnotationsForPretty [] x anns` fixes this problem but breaks a bunch of other test cases. So it looks like this case is failing because the annotations are wrong for some reason.

tfausak commented 2021-11-06 15:02:27 +01:00 (Migrated from github.com)

Dang this just keeps getting more confusing.

Changing layoutDataFamInstDecl to use addAnnotationsForPretty, either building from the existing anns or starting from mempty, does not change the output. That suggests the annotations on the DataFamInstDecl are fine but some annotations outside of it are problematic? I'm not entirely sure.

Here's the difference between the "bad" annotations from this branch and the "good" annotations from the script I posted earlier (https://github.com/lspitzner/brittany/pull/357#issuecomment-962451071):

2a3,11
> ({ TestFakeFileName.hs:1:1 }
>  Just (Ann (DP (0,0)) [] [] [(AnnEofPos,DP (1,0))] Nothing Nothing)
>  (HsModule
>   (VirtualBraces
>    (1))
>   (Nothing)
>   (Nothing)
>   []
>   [
66c75,77
<         []))))))))
---
>            []))))))))]
>   (Nothing)
>   (Nothing)))

The AnnEofPos is strange. I thought it went away? https://github.com/lspitzner/brittany/pull/357#discussion_r740990692

It does live on as a field on ApiAnns: https://downloads.haskell.org/~ghc/9.0.1/docs/html/libraries/ghc-9.0.1/GHC-Parser-Annotation.html#v:apiAnnEofPos

But that doesn't explain how it shows up in ghc-exactprint's output.

Dang this just keeps getting more confusing. Changing `layoutDataFamInstDecl` to use `addAnnotationsForPretty`, either building from the existing `anns` or starting from `mempty`, does not change the output. That suggests the annotations on the `DataFamInstDecl` are fine but some annotations outside of it are problematic? I'm not entirely sure. Here's the difference between the "bad" annotations from this branch and the "good" annotations from the script I posted earlier (https://github.com/lspitzner/brittany/pull/357#issuecomment-962451071): ``` diff 2a3,11 > ({ TestFakeFileName.hs:1:1 } > Just (Ann (DP (0,0)) [] [] [(AnnEofPos,DP (1,0))] Nothing Nothing) > (HsModule > (VirtualBraces > (1)) > (Nothing) > (Nothing) > [] > [ 66c75,77 < [])))))))) --- > []))))))))] > (Nothing) > (Nothing))) ``` The `AnnEofPos` is strange. I thought it went away? https://github.com/lspitzner/brittany/pull/357#discussion_r740990692 It does live on as a field on `ApiAnns`: https://downloads.haskell.org/~ghc/9.0.1/docs/html/libraries/ghc-9.0.1/GHC-Parser-Annotation.html#v:apiAnnEofPos But that doesn't explain how it shows up in `ghc-exactprint`'s output.

tfausak commented 2021-11-06 16:07:26 +01:00 (Migrated from github.com)

I narrowed it down to these lines:

42cf56b106/src/Language/Haskell/Brittany/Internal.hs (L465-L466)

If I comment out the second line, then Brittany correctly prints type families. Unfortunately a bunch of other things break.

I narrowed it down to these lines: https://github.com/lspitzner/brittany/blob/42cf56b1061da92fddbb76085b37abd0f401e2fe/src/Language/Haskell/Brittany/Internal.hs#L465-L466 If I comment out the second line, then Brittany correctly prints type families. Unfortunately a bunch of other things break.

tfausak (Migrated from github.com) reviewed 2021-11-06 16:18:44 +01:00

src/Language/Haskell/Brittany/Internal.hs

tfausak (Migrated from github.com) commented 2021-11-06 16:18:43 +01:00

Aha! AnnEofPos does still exist. It's no longer an AnnKeywordId defined by ghc. Rather it's a KeywordId defined by ghc-exactprint. I was confused by the documentation, which doesn't show it as a constructor. However looking at the source reveals it behind CPP.

Aha! `AnnEofPos` does still exist. It's no longer an `AnnKeywordId` defined by `ghc`. Rather it's a `KeywordId` defined by `ghc-exactprint`. I was confused by [the documentation](https://hackage.haskell.org/package/ghc-exactprint-0.6.4/docs/Language-Haskell-GHC-ExactPrint-Types.html#t:KeywordId), which doesn't show it as a constructor. However looking at [the source](https://hackage.haskell.org/package/ghc-exactprint-0.6.4/docs/src/Language.Haskell.GHC.ExactPrint.Types.html#KeywordId) reveals it behind CPP.

tfausak commented 2021-11-06 16:44:33 +01:00 (Migrated from github.com)

Both #198 and 9236673d66 seem relevant to this problem.

Both #198 and 9236673d66644ebc8f49e897a7b47bf61214c434 seem relevant to this problem.

tfausak (Migrated from github.com) reviewed 2021-11-06 17:47:38 +01:00

src/Language/Haskell/Brittany/Internal.hs

tfausak (Migrated from github.com) commented 2021-11-06 17:47:38 +01:00

These unused comment errors were only showing up in tests for me. If I copy-pasted the test case into a source file and ran the brittany executable against it, I got no warnings or errors.

These unused comment errors were only showing up in tests for me. If I copy-pasted the test case into a source file and ran the `brittany` executable against it, I got no warnings or errors.

tfausak (Migrated from github.com) reviewed 2021-11-06 17:47:58 +01:00

src/Language/Haskell/Brittany/Internal.hs

tfausak (Migrated from github.com) commented 2021-11-06 17:47:58 +01:00

I couldn't tell you why this is necessary, but it does in fact solve the problem I was having.

I couldn't tell you why this is necessary, but it does in fact solve the problem I was having.

tfausak commented 2021-11-06 17:49:19 +01:00 (Migrated from github.com)

Now that this works for GHC 9.0, I need to decide if it's reasonable to also support GHC 8.10 (and earlier). In other words, can these changes be applied to #356?

Now that this works for GHC 9.0, I need to decide if it's reasonable to also support GHC 8.10 (and earlier). In other words, can these changes be applied to #356?

tfausak commented 2021-11-15 14:48:29 +01:00 (Migrated from github.com)

I tested this against my work codebase, which was previously formatted with Brittany 0.13.1.2. This branch didn't change anything, which is great! Everything seems to be working as expected.

I tested this against my work codebase, which was previously formatted with Brittany 0.13.1.2. This branch didn't change anything, which is great! Everything seems to be working as expected.

❤️ 1

tfausak commented 2021-11-28 15:01:18 +01:00 (Migrated from github.com)

73 commits later, here’s where this stands:

• It works with GHC 9.0. All the tests pass. Running the updated executable against various code bases that I maintain appears to work fine.
• It doesn’t work with any older versions of GHC. Removing the CPP made things easier for me to maintain.
• The basic work to get this building with GHC 9.0 wasn’t too obnoxious, but I don’t really want to maintain compatibility with multiple versions of GHC at the same time.

In short: mission accomplished.

I would merge this PR, but I don’t have admin access to this repo. That means I can’t add secrets, which means I can’t automatically release new versions through GitHub Actions. I have all the set up on my fork, so I will probably continue developing it there. We’ll see how that plays out moving forward.

My next goal is to get Brittany building with GHC 9.2. Unfortunately many dependencies don’t yet build with 9.2, so that’s going to be challenging. Although I haven’t looked too closely yet, I suspect my overall plan will involve trying to reduce the number of dependencies. For example I think butcher could be dropped for something like optparse-applicative or even System.Console.GetOpt.

73 commits later, here’s where this stands: - It works with GHC 9.0. All the tests pass. Running the updated executable against various code bases that I maintain appears to work fine. - It _doesn’t_ work with any older versions of GHC. Removing the CPP made things easier for me to maintain. - The basic work to get this building with GHC 9.0 wasn’t too obnoxious, but I don’t really want to maintain compatibility with multiple versions of GHC at the same time. In short: mission accomplished. I would merge this PR, but I don’t have admin access to this repo. That means I can’t add secrets, which means I can’t automatically release new versions through GitHub Actions. I have all the set up on my fork, so I will probably continue developing it there. We’ll see how that plays out moving forward. My next goal is to get Brittany building with GHC 9.2. Unfortunately many dependencies don’t yet build with 9.2, so that’s going to be challenging. Although I haven’t looked too closely yet, I suspect my overall plan will involve trying to reduce the number of dependencies. For example I think `butcher` could be dropped for something like `optparse-applicative` or even `System.Console.GetOpt`.

tfausak commented 2021-11-28 15:42:41 +01:00 (Migrated from github.com)

https://github.com/tfausak/brittany/releases/tag/0.14.0.0

https://github.com/tfausak/brittany/releases/tag/0.14.0.0

❤️ 1

arybczak commented 2021-11-29 09:33:02 +01:00 (Migrated from github.com)

Thank you for the work! Supporting a single compiler version looks like the right move, it's a bit sad that maintaining compatibility for multiple GHC API versions is so hard right now.

Thank you for the work! Supporting a single compiler version looks like the right move, it's a bit sad that maintaining compatibility for multiple GHC API versions is so hard right now.

jneira commented 2022-01-02 17:59:56 +01:00 (Migrated from github.com)

@tfausak many thanks for your work with this, hls will be able to enable brittany for ghc-9.0.1 thanks to.
Sadly the new hackage version 0.14 mixed two important breaking changes: support for aeson >= 2.0 (only) and ghc == 9.0.1.
Due to that combination we can't jump in hls to aeson 2.0 for ghc < 9.0.1 and we will have to keep compatibility for aeson < 2.0 until we drop support for ghc 8.10, so for a long time (or droping the brittany plugin)
Any chance to release a intermmediate version with support for ghc < 9.0.1 and aeson-2.0? thanks!

@tfausak many thanks for your work with this, hls will be able to enable brittany for ghc-9.0.1 thanks to. Sadly the new hackage version 0.14 mixed two important breaking changes: support for aeson >= 2.0 (only) and ghc == 9.0.1. Due to that combination we can't jump in hls to aeson 2.0 for ghc < 9.0.1 and we will have to keep compatibility for aeson < 2.0 until we drop support for ghc 8.10, so for a long time (or droping the brittany plugin) Any chance to release a intermmediate version with support for ghc < 9.0.1 *and* aeson-2.0? thanks!

tfausak commented 2022-01-03 02:34:26 +01:00 (Migrated from github.com)

I'll have to see what changes I'd need to make to support Aeson 2. From what I remember there wasn't too much to do there.

I'll have to see what changes I'd need to make to support Aeson 2. From what I remember there wasn't too much to do there.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

blocked: dependency

  

blocked: info-needed

  

bug

  

duplicate

  

enhancement

  

fixed in HEAD

  

help wanted

  

hs:arrows

  

hs:brackets

  

hs:classes

  

hs:comments

  

hs:do-notation

  

hs:guards

  

hs:lists

  

hs:operators

  

hs:patterns

  

hs:records

  

hs:types

  

invalid

  

language extension support

  

layouting

  

needs confirmation

  

priority: high

  

priority: low

  

question

  

revisit before next release

  

wontfix

No Label

blocked: dependency

blocked: info-needed

bug

duplicate

enhancement

fixed in HEAD

help wanted

hs:arrows

hs:brackets

hs:classes

hs:comments

hs:do-notation

hs:guards

hs:lists

hs:operators

hs:patterns

hs:records

hs:types

invalid

language extension support

layouting

needs confirmation

priority: high

priority: low

question

revisit before next release

wontfix

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#357

There is no content yet.