GrammaticalFramework / gf-core

Grammatical Framework core: compiler, shell & runtimes
https://www.grammaticalframework.org
Other
131 stars 35 forks source link

prepare for GHC 9, base 4.15, by using Buffer constructor interface #135

Closed mengwong closed 2 years ago

mengwong commented 2 years ago

base 4.15 added a bufOffset field to the Buffer record.

Without this patch, we can expect this error when compiling with GHC 9

gf      > /private/var/folders/dk/jgh48hw9113b8fzq0qrs3q9c0000gp/T/stack-74d3cbf985184a8c/gf-3.11/src/compiler/GF/Text/Coding.hs:41:16: error:
gf      >     • Constructor ‘Buffer’ does not have the required strict field(s): bufOffset
gf      >     • In the expression:
gf      >         Buffer
gf      >           {bufRaw = fptr, bufState = ReadBuffer, bufSize = len, bufL = l,
gf      >            bufR = l + len}
gf      >       In an equation for ‘bbuf’:
gf      >           bbuf
gf      >             = Buffer
gf      >                 {bufRaw = fptr, bufState = ReadBuffer, bufSize = len, bufL = l,
gf      >                  bufR = l + len}
gf      >       In the expression:
gf      >         do let bbuf = ...
gf      >            cbuf <- newCharBuffer 128 WriteBuffer
gf      >            case enc of { TextEncoding {mkTextDecoder = mk} -> do ... }
gf      >    |
gf      > 41 |     let bbuf = Buffer{bufRaw=fptr, bufState=ReadBuffer, bufSize=len, bufL=l, bufR=l+len}
gf      >    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
mengwong commented 2 years ago

emptyBuffer initializes bufOffset to 0. I hope that's okay – I don't really know what the code is doing so this patch is purely syntactic.

inariksit commented 2 years ago

Thank you @mengwong! The build fails on ghc 7.10.3, and the later checks are skipped, so it's unclear whether they work. Did you check this with some of the ghc 8.* versions?

inariksit commented 2 years ago

Seems like the failing is due to something else. The build has failed since e4b2f28, and this patch succeeds on my computer with stack install for several ghc versions, and test suit passes. So I suggest we merge this, unless there are objections from @johnjcamilleri or @krangelov?

johnjcamilleri commented 2 years ago

So this pull request is effectively breaking support for GHC < 8? Is there no way to put this behind a compiler version pragma, as is done elsewhere in the code?

inariksit commented 2 years ago

@johnjcamilleri No it doesn't seem to be actually breaking, it works on my computer for GHC < 8. The breaking happened in a previous commit (e4b2f28), which suggests that something has changed on the build server. All of these breaking commits have worked on my computer.

inariksit commented 2 years ago

Now that #138 fixed the issue on build server, we can see that this works on all ghc versions we test for.