Skip to content

Conversation

@tfausak
Copy link
Collaborator

@tfausak tfausak commented Nov 15, 2020

Continuing work from #312.

@tfausak
Copy link
Collaborator Author

tfausak commented Nov 15, 2020

Now there is a legitimate test failure against GHC 8.10. It's related to #48. I don't know enough to fix it.

  src-literatetests/Main.hs:183:3: 
  1) regression issue 48 a
       expected: Right 
                 foo =
                   let a    = b@1
                       cccc = ()
                   in  foo
                 
        but got: Right 
                 foo =
                   let a    = _
                       cccc = ()
                   in  foo
                 

  To rerun use: --match "/regression/issue 48 a/"

  src-literatetests/Main.hs:183:3: 
  2) context free: regression issue 48 a
       expected: Right 
                 foo =
                   let
                     a = b@1
                     cccc = ()
                   in foo
                 
        but got: Right 
                 foo =
                   let
                     a = _
                     cccc = ()
                   in foo

https://github.com/lspitzner/brittany/pull/324/checks?check_run_id=1403080234#step:7:1202

@tfausak tfausak mentioned this pull request Nov 15, 2020
@expipiplus1
Copy link
Contributor

Now there is a legitimate test failure against GHC 8.10. It's related to #48. I don't know enough to fix it.

Looking at --dump-full-ast from brittany with 8.8 and brittany with 8.10, we can see a difference:

where 8.8 returns:

[ A Just (Ann (DP (0,-1)) [] [] [] Nothing Nothing)
    GRHS
      NoExt
      []
      A Just (Ann (DP (0,1)) [] [] [((G AnnAt),DP (0,0))] Nothing Nothing)
        EAsPat
          NoExt
          A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
            Unqual {OccName: b}
          A Just (Ann (DP (0,0)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
            HsOverLit
              NoExt
              OverLit
                NoExt
                HsIntegral
                  IL
                    SourceText "1"
                    False
                    1
                HsLit
                  NoExt
                  HsString
                    SourceText "noExpr"
                    {FastString: "noExpr"}
]

8.10 returns a hole!

[ A Just (Ann (DP (0,-1)) [] [] [] Nothing Nothing)
    GRHS
      NoExtField
      []
      A Just (Ann (DP (0,1)) [] [] [((G AnnVal),DP (0,0))] Nothing Nothing)
        HsUnboundVar
          NoExtField
          TrueExprHole {OccName: _}
]

It's also not valid syntax as ghci tells us:

λ> b = a@()

<interactive>:3:5: error:
    Pattern syntax in expression context: a@()
    Type application syntax requires a space before '@'

I think it's fine to remove this test (or correct it so that it has the space before the @)

@expipiplus1
Copy link
Contributor

It's surprising to me that the AST can even represent this, although perhaps it's useful for error messages or something

@tfausak
Copy link
Collaborator Author

tfausak commented Nov 20, 2020

or correct it so that it has the space before the @

👍

@expipiplus1
Copy link
Contributor

Need to update the supported GHC versions in the readme

@tfausak
Copy link
Collaborator Author

tfausak commented Nov 23, 2020

I should make sure there are stack.yaml files for each configuration. Then I think this will be ready to go.

@expipiplus1
Copy link
Contributor

@tfausak would you like any help with this release?

@tfausak
Copy link
Collaborator Author

tfausak commented Dec 9, 2020

Sorry! Thanks for the ping. I don't need any help with this, I just have to actually do it. With any luck I should be able to spend some time on it in the next few days.

@tfausak tfausak merged commit 6346e20 into master Dec 10, 2020
@tfausak tfausak deleted the gh-269-ghc-8.10 branch December 10, 2020 02:50
@tfausak tfausak mentioned this pull request Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants