Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reenable -fpure-haskell #554

Merged
merged 2 commits into from
Jan 27, 2024
Merged

Conversation

chreekat
Copy link
Contributor

The other implementation looks identical:

reverseNonEmpty (Text (A.ByteArray ba) off len) = runST $ do
    marr@(A.MutableByteArray mba) <- A.new len
    unsafeIOToST $ c_reverse mba ba (fromIntegral off) (fromIntegral len)
    brr <- A.unsafeFreeze marr
    return $ Text brr 0 len

This function was, in parallel, reimplemented in
Data.Text.Internal.Reverse. This instance appears unused, since the
testsuite still passes.
@chreekat chreekat mentioned this pull request Jan 27, 2024
@TeofilC
Copy link
Contributor

TeofilC commented Jan 27, 2024

Thanks for spotting and fixing this!

@Bodigrim
Copy link
Contributor

I'm somewhat out of brain power today: how does removing a bunch of code not break anything but instead fixes something? ;)

@chreekat
Copy link
Contributor Author

Understandable! :D

It looks like Teo moved reverseNonEmpty from its original location into a new module. I did the same thing, but into a different module. My version also got reimplemented to work with -fpure-haskell, but Teo's did not. Finally, by the mysterious ways of git, it's my reverseNonEmpty that is imported into Data.Text.Lazy.

Thus, Teo's Data.Text.Internal.Transformation.reverseNonEmpty is unused within the library itself, and fails to compile with -fpure-haskell. Since it is identical to my function (under -fno-pure-haskell, that is), removing it fixes the problem.

@Bodigrim Bodigrim requested a review from Lysxia January 27, 2024 12:10
@Lysxia Lysxia merged commit 8ea29aa into haskell:master Jan 27, 2024
27 checks passed
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.

4 participants