Skip to content

Conversation

@Shimuuar
Copy link
Contributor

This is a simplification of rewrite rules: use type application for compactness and group them all together.

Also I found that one of them is wrong. Rule clearly intended for 32-bit builds was enabled for 64-bit instead.

#if WORD_SIZE_IN_BITS > 32
-- FIXME: the "too large" test is totally wrong
enumFromTo_big_int :: forall m v a. (HasCallStack, Integral a, Monad m) => a -> a -> Bundle m v a
{-# INLINE_FUSED enumFromTo_big_int #-}
enumFromTo_big_int x y = x `seq` y `seq` fromStream (Stream step (Just x)) (Exact (len x y))
where
{-# INLINE [0] len #-}
len :: HasCallStack => a -> a -> Int
len u v | u > v = 0
| otherwise = check Bounds "vector too large"
(n > 0 && n <= fromIntegral (maxBound :: Int))
$ fromIntegral n
where
n = v-u+1
{-# INLINE_INNER step #-}
step Nothing = return $ Done
step (Just z) | z == y = return $ Yield z Nothing
| z < y = return $ Yield z (Just (z+1))
| otherwise = return $ Done
{-# RULES
"enumFromTo<Int64> [Bundle]"
enumFromTo = enumFromTo_big_int :: Monad m => Int64 -> Int64 -> Bundle m v Int64 #-}
#endif

Compare

#if WORD_SIZE_IN_BITS > 32
"enumFromTo<Int64> [Bundle]"
enumFromTo = enumFromTo_intlike :: Monad m => Int64 -> Int64 -> Bundle m v Int64 #-}
#else

That made me think. We notionally support 32-bit platforms, we have CPP for them, but we don't even test buildability on CI. Does anyone has ideas how to add such tests?

Note that #ifdef condition was wrong! We had 2 rules on 64 bits and none at 32
bit.

Lesson: #ifdef spaghetti is bad. It's better to keep conditions few and local.
@konsumlamm
Copy link
Contributor

That made me think. We notionally support 32-bit platforms, we have CPP for them, but we don't even test buildability on CI. Does anyone has ideas how to add such tests?

GitHub actions provide an i386 container, see for example https://github.com/Bodigrim/bitvec/blob/95cdcf0c046b94af284ab4e7903da4d435fbf5c5/.github/workflows/ci.yaml#L49-L67. It's also possible to emulate various architectures (see below that), but that is significantly slower. You could also consider testing the JS and WASM backends (see for example https://github.com/Bodigrim/bitvec/blob/95cdcf0c046b94af284ab4e7903da4d435fbf5c5/.github/workflows/ci.yaml#L96-L159).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants