Move slice to Partial, add sliceMaybe function

[bjornbm]

Closes #200.

[snoyberg]

Some quickchecks/property tests would be a great addition here.

[bjornbm]

Ah, I didn't write any because I didn't see any in ListSpec.hs.

[bjornbm]

I was about to replace all the tests with:

spec :: Spec
spec =
  describe "sliceMaybe" $ do
    prop "is consistent with `slice` (pathological cases)" $
      \(QC.Large i) (QC.Large n) v
        -> sliceProp i n (DV.fromList v)
    prop "is consistent with `slice` (less pathological cases)" $
      \(QC.NonNegative i) (QC.NonNegative n) (QC.NonEmpty v)
        -> sliceProp i n (DV.fromList v)
  where
    sliceProp :: Int -> Int -> DV.Vector Char -> QC.Property
    sliceProp i n v = QC.withMaxSuccess 10000 $ case VB.sliceMaybe i n v of
      Just v' -> DV.slice i n v `shouldBe` v'
      Nothing -> evaluate (DV.slice i n v) `shouldThrow` anyException

(The second property could be skipped since it is implicit in the first and the number of cases could be reduced to the default of 100, but the coverage may not be extensive.)

However, these tests resulted in:

rio        > RIO.Vector       
rio        >   sliceMaybe     
rio        >     is consistent with `slice` (pathological cases) FAILED [1]
rio        >     is consistent with `slice` (less pathological cases)
rio        >       +++ OK, passed 10000 tests.
rio        >                  
rio        > Failures:        
rio        >                  
rio        >   test/RIO/VectorSpec.hs:23:18: 
rio        >   1) RIO.Vector.sliceMaybe is consistent with `slice` (pathological cases)
rio        >        Falsifiable (after 82 tests and 25 shrinks):
rio        >          Large {getLarge = 2104973872246037021}
rio        >          Large {getLarge = 7118398164608738787}
rio        >          ""      
rio        >        did not get expected exception: SomeException

Further investigation led me to open https://gitlab.haskell.org/ghc/ghc/issues/17233.

[snoyberg]

What makes you think this is a GHC bug instead of a vector library bug?

[bjornbm]

I just used the heuristic that if it is a segfault or the error message says Please report this as a GHC bug it should be reported here. But it may well be (probably is?) a vector bug so I reported it as haskell/vector#257 too.

[bjornbm]

Take a look at 0e59957 and 8673eb7 see if you like any of the approaches.

[snoyberg]

I'd rather see a patch like this land in vector than here. I'm also concerned about surprising behavior of error types changing.

[bjornbm]

OK, I will remove 8673eb7 then.

[snoyberg]

I don't think it's necessary to replace the unit tests with property tests, I was just hoping to include property tests as well.

[bjornbm]

That was understood. However, I felt the property test was more to the point and completely specified the intended behavior of sliceMaybe (in terms of slice), thereby making the unit tests redundant.

Things became a bit complicated with the revealed bug of course.

[snoyberg]

Since we're doing the checking ourselves, moving to unsafeSlice is a good idea.

[lehins]

slice has finally been fixed in vector-0.12.1 haskell/vector#257

[akhra]

@bjornbm I've got your tests passing locally with current vector functions (removing local RIO.Vector.Partial.slice), are you still active/interested here? if not I'll open a separate PR to avoid wrangling multiple forks.

[bjornbm]

Hi @akhra, please do take over! 🙏

[akhra]

moved to local branch with #281