Support GHC 9.2

[fumieval]

I confirmed that this compiles with the following cabal.project:

packages:
    ./warp/
    ./wai/

allow-newer:
  wuss:base,
  cereal:base,
  basement:base,
  websockets:bytestring,
  wuss:bytestring,
  aeson:base,
  hsc2hs:base,

source-repository-package
  type: git
  location: git@github.com:fpco/streaming-commons.git
  tag: ac8c695

source-repository-package
  type: git
  location: https://github.com/ekmett/lens.git
  tag: 211c2fbbea8fb64a7f3f72d709a95f0e2e3995b8

source-repository-package
  type: git
  location: https://github.com/kazu-yamamoto/iproute
  tag: 1acc17af9cf770fdbabf55fcb7a949939efe0af4

source-repository-package
  type: git
  location: https://github.com/kazu-yamamoto/iproute
  tag: 1acc17af9cf770fdbabf55fcb7a949939efe0af4

source-repository-package
  type: git
  location: https://github.com/TomMD/foundation
  tag: 0bb195e1fea06d144dafc5af9a0ff79af0a5f4a0
  subdir: basement

source-repository-package
  type: git
  location: https://github.com/fumieval/hs-memory
  tag: 7d385cecad6d496ab0dc079237ed133b5de43b2d

source-repository-package
  type: git
  location: https://github.com/josephcsible/cryptonite
  tag: 3b081e3ad027b0550fc87f171dffecbb20dedafe

---

Before submitting your PR, check that you've:

• Bumped the version number
• Documented new APIs with Haddock markup
• Added @since declarations to the Haddock

After submitting your PR:

• Update the Changelog.md file with a link to your PR
• Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

[kazu-yamamoto]

After applying this patch, the tests with GHC 8.10.7 gets failed. Would you give a look?

You don't have to worry about the failures on GHC 9.0 or 9.2. They are due to GHC 9.x.

[fumieval]

Can you show me an example or trigger the CI? I'm not sure if I follow what you mean by "due to GHC 9.x"

[fumieval]

The code smells so I just replaced the implementation with something more straightforward. Has mhDigitToInt been benchmarked and proven to be faster? I'd be very surprised if it is better than subtraction...

[kazu-yamamoto]

I'm testing this patch on my local machine.
cabal test with GHC 8.10.7 fails after applying your patch.

For GHC 9, even master fails. I experienced this kind of failures with many network-related libraries.

[fumieval]

I was asking which test is failing and how, not what your environment is, but it's OK as I was able to reproduce it too.

Out of curiousity, could you tell me what kind of failure are you experiencing on GHC 9.0 with concrete examples?

[kazu-yamamoto]

First of all, this is the result of cabal test with GHC 8.10.7:

Failures:

  test/ReadIntSpec.hs:14:5: 
  1) ReadInt.readInt64 converts ByteString to Int
       Falsified (after 1 test):
         0

  To rerun use: --match "/ReadInt/readInt64/converts ByteString to Int/"

  test/RunSpec.hs:166:24: 
  2) Run.non-pipelining has body, read
       expected: 2
        but got: 0

  To rerun use: --match "/Run/non-pipelining/has body, read/"

  test/RunSpec.hs:166:24: 
  3) Run.non-pipelining has body, ignore
       expected: 2
        but got: 1

  To rerun use: --match "/Run/non-pipelining/has body, ignore/"

  test/RunSpec.hs:166:24: 

[fumieval]

Right, I saw it too. Since the old implementation doesn't make much sense to me, I replaced mhDigitToInt with x - 48. Can you confirm if it works?

[fumieval]

Also can you trigger the CI? Workflows don't run unless a maintainer approves

[kazu-yamamoto]

To run CI.

[fumieval]

@kazu-yamamoto by approval, I meant approving workflow triggers, not the changes. See also https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

[fumieval]

TBH I have no idea how it has been working before. It should have been like this:

--- a/warp/Network/Wai/Handler/Warp/ReadInt.hs
+++ b/warp/Network/Wai/Handler/Warp/ReadInt.hs
@@ -37,13 +37,14 @@ readInt64 :: ByteString -> Int64
 readInt64 bs = S.foldl' (\ !i !c -> i * 10 + fromIntegral (mhDigitToInt c)) 0
              $ S.takeWhile isDigit bs
 
-data Table = Table !Addr#
+data Table = Table Addr#
 
 {-# NOINLINE mhDigitToInt #-}
 mhDigitToInt :: Word8 -> Int
-mhDigitToInt (W8# i) = I# (word2Int# (indexWord8OffAddr# addr (word2Int# i)))
-  where
-    !(Table addr) = table
+mhDigitToInt (W8# i) = case table of
+  Table addr -> I# (word2Int# (word8ToWord# (indexWord8OffAddr# addr (word2Int# (word8ToWord# i)))))
+
+{-# NOINLINE table #-}
 table :: Table
 table = Table
     "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\

But of course it's a good practice not to write code like this.

[kazu-yamamoto]

@fumieval Sorry for my confusing. I noticed it after I approved the change. X-(

[kazu-yamamoto]

If I remember correctly, ! is the secret of this code.

[kazu-yamamoto]

I agree that your code is much easier to understand.

[kazu-yamamoto]

Talking about GHC 9.x. cabal test never finish at least for quic and warp. When I run spec manually, I saw some errors and the test stops.

[fumieval]

warp's test seems to pass after my - 48 change

[kazu-yamamoto]

What kind of OS do you use?

[fumieval]

Ubuntu 21.04, GHC 9.2.1, Cabal 3.6.2.0, Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz

I can imagine that the old implementation is so fragile that it could get stuck or segfault in the worst case.

[kazu-yamamoto]

On my monterey, the current this PR works well with GHC 8.10.7.
But with GHC 9.0 and 9.2, cabal test stops:

Run
  non-pipelining
    no body, read
    no body, ignore
    has body, read
    has body, ignore
    chunked body, read
    chunked body, ignore
  pipelining
    no body, read

No progression after here.
But I know this is not due this PR. This is due to GHC 9.

[kazu-yamamoto]

@snoyberg If I remember correctly this code is written by Matt Brown. @fumieval wishes to replace it with non-magical code. What do you think?

[snoyberg]

No objection from me at all, I don't remember the code

[kazu-yamamoto]

Merged. Thank you, guys!

[fumieval]

I forgot to push the change but the bang patterns in foldl' are redundant (regardless of the Strict extension)

[kazu-yamamoto]

@fumieval Would you send another PR?

[kazu-yamamoto]

A new version of Warp has been released for GHC 9.2!