Skip to content

Conversation

@konsumlamm
Copy link
Contributor

Fix the bug mentioned in #265 by generating different functions for Float and Double that check x == 0.0 && !signbit(x) to ensure that we only use memset for +0.0 (and not -0.0).

I also removed the special case for sizeof(Hs ## TYPE) == sizeof(int)*2 since it didn't yield better performance in my benchmark. I also don't see why one would expect performance improvements from that in the first place, since a single access should be faster than two.

Add tests for `Float` & `Double`
@andrewthad
Copy link
Collaborator

This looks correct to me. And also, it looks like the tests suite would catch a mistake in the implementation of Prim for Float or Double. Are you alright with me merging this?

@konsumlamm
Copy link
Contributor Author

And also, it looks like the tests suite would catch a mistake in the implementation of Prim for Float or Double.

Yes, I added primLaw tests for Float and Double since they were missing. I also added separate tests for the -0.0 bug, since as far as I can tell the Arbitrary instances don't generate -0.0.

Are you alright with me merging this?

Yes, go ahead!

@andrewthad andrewthad merged commit 02f2546 into haskell:master Dec 17, 2025
13 checks passed
@konsumlamm konsumlamm deleted the fix branch December 17, 2025 13:29
@treeowl
Copy link
Collaborator

treeowl commented Dec 17, 2025

Yes, I added primLaw tests for Float and Double since they were missing. I also added separate tests for the -0.0 bug, since as far as I can tell the Arbitrary instances don't generate -0.0.

Should that be considered a bug in QuickCheck? Has it been reported?

@konsumlamm
Copy link
Contributor Author

Yes, I added primLaw tests for Float and Double since they were missing. I also added separate tests for the -0.0 bug, since as far as I can tell the Arbitrary instances don't generate -0.0.

Should that be considered a bug in QuickCheck? Has it been reported?

0.0 and -0.0 behave the same in most cases and == can't distinguish them, but I think it would still make sense to generate it. It is mentioned in nick8325/quickcheck#98.

@treeowl
Copy link
Collaborator

treeowl commented Dec 17, 2025

That QuickCheck ticket is old enough to go to kindergarten.

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