binary icon indicating copy to clipboard operation
binary copied to clipboard

Alternative instance for Get does not respect identity law in error situations

Open sugarbleat opened this issue 3 years ago • 0 comments

Hello,

While trying to combine Get parsers using asum, I found out that instance Alternative Get is not lawful: x <|> empty is not the same as x with respect to failure. x <|> empty errors with "Data.Binary.Get(Alternative).empty", overriding the error message from x.

For me, an unfortunate consequence is that the implementation of asum leaks through: asum [x, y] = x <|> y <|> empty provides less helpful parse errors than x <|> y.

I have attached a minimal working example, also hosted on this Gist: https://gist.github.com/sugarbleat/4f30751feedf8d3e06911deae7ef4a5a, which can be run with cabal run Main.hs. For reference, a similar test with Parsec shows no violation of identity.

Thanks for the great work on the library, by the way!

#!/usr/bin/env cabal
{- cabal:
build-depends: base ^>= 4.15
             , bytestring ^>= 0.10.12
             , binary == 0.8.9.1
-}
module Main (main) where

import Control.Applicative (Alternative (..))
import Data.Binary (Get, Word8, get)
import Data.Binary.Get (runGetOrFail)
import qualified Data.ByteString.Lazy as B
import Data.Foldable (asum)

testGet :: Show a => Get a -> B.ByteString -> IO ()
testGet p s = do
  putStrLn "p"
  print $ runGetOrFail p s

  putStrLn "p <|> empty"
  print $ runGetOrFail (p <|> empty) s

  putStrLn "asum [p]"
  print $ runGetOrFail (asum [p]) s

main :: IO ()
main = testGet (get :: Get Word8) B.empty

{-
Output:
  p
  Left ("",0,"not enough bytes")
  p <|> empty
  Left ("",0,"Data.Binary.Get(Alternative).empty")
  asum [p]
  Left ("",0,"Data.Binary.Get(Alternative).empty")
-}

sugarbleat avatar Oct 10 '22 20:10 sugarbleat