Skip to content

New instances and CI cleanup #427

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

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open

New instances and CI cleanup #427

wants to merge 35 commits into from

Conversation

MaximilianAlgehed
Copy link
Collaborator

No description provided.

@MaximilianAlgehed
Copy link
Collaborator Author

@nick8325 @UlfNorell and @Rewbert I'd like to get some input on the state of the RunCollectDataTypes stuff. I've made a bunch of changes, added a bunch of instances of Arbitrary, and ignored a fair number of both data-types and whole modules for now to get things to build (and run!). If you guys wouldn't mind having a look at the state of this now and maybe trying to add a few more types and modules back in that would be great!

@Rewbert
Copy link
Collaborator

Rewbert commented Jun 12, 2025

Looks really nice! I will try to add some instances to get more familiar with it.

@MaximilianAlgehed This PR will stay open for a while, right? Or will we squeeze all our proposed changes into different PRs?

@MaximilianAlgehed
Copy link
Collaborator Author

@Rewbert maybe the best thing to do right now is to move the HackingAgenda.md file into an issue and trying to make this PR just the instances stuff so that we at least have that on master?

@MaximilianAlgehed MaximilianAlgehed changed the title Hacking New instances and CI cleanup Jun 12, 2025
@Rewbert
Copy link
Collaborator

Rewbert commented Jun 12, 2025

Makes sense. I hope to find some time tomorrow to look at more instances :)

@Rewbert
Copy link
Collaborator

Rewbert commented Jun 13, 2025

Pushed some silly instance for System.Console. Please see @MaximilianAlgehed

It exports only three types, and it is trivial to derive instances

instance Arbitrary (ArgDescr Int) where
  arbitrary = oneof [ NoArg <$> arbitrary
                    , ReqArg <$> arbitrary <*> arbitrary
                    , OptArg <$> arbitrary <*> arbitrary
                    ]

instance Arbitrary (ArgOrder Int) where
  arbitrary = oneof [ return RequireOrder
                    , return Permute
                    , ReturnInOrder <$> arbitrary
                    ]

instance Arbitrary (OptDescr Int) where
  arbitrary = Option
                <$> arbitrary
                <*> arbitrary
                <*> arbitrary
                <*> arbitrary

But it is very unlikely that anything sensible will be generated. However, anything sensitive will be dependent on some context, which we cannot supply lmao. I am not sure that I am a fan of these instances.

Had a look at System.Posix as well. It exports a buttload of types. I am sure that we can provide instances for some of them, but not all. Some types are e.g. created via CType macros (never used these macros personally before).

@MaximilianAlgehed
Copy link
Collaborator Author

These are fine AFAIK, but it would be nice with shrinking and CPP pragmas to make sure the build passes! ;)

@Rewbert
Copy link
Collaborator

Rewbert commented Jun 13, 2025

Haha right, I will make sure to fix it.

@MaximilianAlgehed
Copy link
Collaborator Author

CI won't trigger because there are conflicts I think.

@Rewbert
Copy link
Collaborator

Rewbert commented Jun 16, 2025

Looks like a silly conflict!

@Rewbert
Copy link
Collaborator

Rewbert commented Jun 16, 2025

System.Console appears to have been in base since forever, so leaving the CPP out completely will also be just fine. These bounds specify that it should work for every base/GHC version that we support.

Copy link
Contributor

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the revision.

Looks like some consequences of base >= 4.14 have not been pushed through yet...

QuickCheck.cabal Outdated
@@ -136,17 +134,6 @@ library
if !impl(ghc >= 7.4)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional can also be deleted, as ghc >= 7.4 will never be false.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically, ghc >= 7.4 is false when you're not compiling for ghc?

QuickCheck.cabal Outdated
-- Safe Haskell appeared in GHC 7.2, but GHC.Generics isn't safe until 7.4.
if impl (ghc < 7.4)
cpp-options: -DNO_SAFE_HASKELL

-- random is explicitly Trustworthy since 1.0.1.0
-- similar constraint for containers
if impl(ghc >= 7.2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make unconditional (and some more below).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was confused by this I think when I went over these impl(ghc >= 7.2) is not true if you're running on hugs or another compiler so I think the right thing to do would be to change these to impl(ghc).

import qualified Data.Semigroup as Semigroup
import Data.Ord

#if MIN_VERSION_base(4,14,0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always true, so you can make this unconditional.

@@ -1035,6 +1089,158 @@ instance Arbitrary (f a) => Arbitrary (Monoid.Alt f a) where
arbitrary = fmap Monoid.Alt arbitrary
shrink = map Monoid.Alt . shrink . Monoid.getAlt
#endif

#if MIN_VERSION_base(4,9,0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always true.

shrink = map Down . shrink . getDown
#endif

#if MIN_VERSION_base(4,14,0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants