well-typed / cborg

Binary serialisation in the CBOR format
https://hackage.haskell.org/package/cborg
191 stars 86 forks source link

Compatibility with ghc-prim-0.9.0.0 on 64 bit #304

Closed alexbiehl closed 1 year ago

alexbiehl commented 2 years ago

Without this patch the build fails with ghc-9.4.1 on my machine. Places for word16 and word32 look very similar already.

centromere commented 1 year ago

@bgamari, @parsonsmatt, are you able to take a look at this? I am unable to build cborg on my Raspberry Pi 4:

cborg> [ 5 of 14] Compiling Codec.CBOR.Magic ( src/Codec/CBOR/Magic.hs, dist/build/Codec/CBOR/Magic.o, dist/build/Codec/CBOR/Magic.dyn_o )
cborg> src/Codec/CBOR/Magic.hs:260:20: error:
cborg>     • Couldn't match expected type ‘Word64#’ with actual type ‘Word#’
cborg>     • In the first argument of ‘W64#’, namely ‘(toWord w#)’
cborg>       In the expression: W64# (toWord w#)
cborg>       In an equation for ‘w64’: w64 w# = W64# (toWord w#)
cborg>     |
cborg> 260 |     w64 w# = W64# (toWord w#)
cborg>     |                    ^^^^^^^^^
cborg> [ 7 of 14] Compiling Codec.CBOR.Encoding[boot] ( src/Codec/CBOR/Encoding.hs-boot, dist/build/Codec/CBOR/Encoding.o-boot, dist/build/Codec/CBOR/Encoding.dyn_o-boot )
cborg> [ 8 of 14] Compiling Codec.CBOR.FlatTerm[boot] ( src/Codec/CBOR/FlatTerm.hs-boot, dist/build/Codec/CBOR/FlatTerm.o-boot, dist/build/Codec/CBOR/FlatTerm.dyn_o-boot )
cborg> [ 9 of 14] Compiling Codec.CBOR.Encoding ( src/Codec/CBOR/Encoding.hs, dist/build/Codec/CBOR/Encoding.o, dist/build/Codec/CBOR/Encoding.dyn_o )
error: builder for '/nix/store/xv84g87g6hhwr3zxrs2djwahlymqcmb4-cborg-0.2.8.0.drv' failed with exit code 1;
       last 10 log lines:
       >     • Couldn't match expected type ‘Word64#’ with actual type ‘Word#’
       >     • In the first argument of ‘W64#’, namely ‘(toWord w#)’
       >       In the expression: W64# (toWord w#)
       >       In an equation for ‘w64’: w64 w# = W64# (toWord w#)
       >     |
       > 260 |     w64 w# = W64# (toWord w#)
       >     |                    ^^^^^^^^^
       > [ 7 of 14] Compiling Codec.CBOR.Encoding[boot] ( src/Codec/CBOR/Encoding.hs-boot, dist/build/Codec/CBOR/Encoding.o-boot, dist/build/Codec/CBOR/Encoding.dyn_o-boot )
       > [ 8 of 14] Compiling Codec.CBOR.FlatTerm[boot] ( src/Codec/CBOR/FlatTerm.hs-boot, dist/build/Codec/CBOR/FlatTerm.o-boot, dist/build/Codec/CBOR/FlatTerm.dyn_o-boot )
       > [ 9 of 14] Compiling Codec.CBOR.Encoding ( src/Codec/CBOR/Encoding.hs, dist/build/Codec/CBOR/Encoding.o, dist/build/Codec/CBOR/Encoding.dyn_o )
       For full logs, run 'nix log /nix/store/xv84g87g6hhwr3zxrs2djwahlymqcmb4-cborg-0.2.8.0.drv'.
parsonsmatt commented 1 year ago

Are you building with this branch? If so, it seems that the ghc-prim version is less than the CPP would indicate.

If not, what version of GHC are you using?

centromere commented 1 year ago

@parsonsmatt, Here are some of the relevant details:

flake.nix snippet:

       overlay = final: prev: {
          haskellPackages = prev.haskell.packages.ghc944.override {
            overrides = hsFinal: hsPrev: {
              cborg = hsPrev.cborg.overrideAttrs (old: {
                doHaddock = false;
                # See: https://github.com/well-typed/cborg/pull/304
                patches = ./patches/cborg-ghc944.patch;
              });
            };
          };
        };

cborg-ghc944.patch:

diff --git a/src/Codec/CBOR/Magic.hs b/src/Codec/CBOR/Magic.hs
index 1b497a2..5875ae4 100644
--- a/src/Codec/CBOR/Magic.hs
+++ b/src/Codec/CBOR/Magic.hs
@@ -257,7 +257,13 @@ grabWord64 (Ptr ip#) =
 #endif

 #if WORD_SIZE_IN_BITS == 64
+
+#if MIN_VERSION_ghc_prim(0,9,0)
+    w64 w# = W64# (wordToWord64# (toWord w#))
+#else
     w64 w# = W64# (toWord w#)
+#endif
+
 #else
     w64 w# = W64# (wordToWord64# (toWord w#))
 #endif

In this configuration, everything builds properly.

parsonsmatt commented 1 year ago

What configuration failed to build? I'm guessing GHC 9.4.4 without this patch?

centromere commented 1 year ago

Correct: When using GHC 9.4.4, this patch is required for my application to build.

tfausak commented 1 year ago

This fixed the build for me with GHC 9.4.4 on an ARM64 machine running Linux. Thanks!

alexbiehl commented 1 year ago

Yeah, would be great if we could merge this. We are running off of a fork for quite some time now.

kubek2k commented 1 year ago

Hey @alexbiehl - I think this is a bit off. If you look at parameters needed for W64# in base-4.17 https://hackage.haskell.org/package/base-4.17.0.0/docs/GHC-Word.html#v:W64-35- and base-4.16 https://hackage.haskell.org/package/base-4.16.0.0/docs/GHC-Word.html#v:W64-35-, you will see that this is the basis of this issue. I think the solution should look like:

 #if WORD_SIZE_IN_BITS == 64
+#if MIN_VERSION_base(4,17,0)
+-- case taken from Codec.CBOR.Decoding
+    w64 w# = W64# (wordToWord64# (toWord w#))
+#else
     w64 w# = W64# (toWord w#)
+#endif
 #else
     w64 w# = W64# (wordToWord64# (toWord w#))
 #endif
-
 #endif

I based my thinking on similar code done in https://github.com/well-typed/cborg/blob/master/cborg/src/Codec/CBOR/Decoding.hs#L331

PR: https://github.com/well-typed/cborg/pull/307

bgamari commented 1 year ago

Fixed in #307.

centromere commented 1 year ago

Thank you @bgamari!