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.

Sign up for GitHub

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

Jump to bottom

Add instances for SNat, SSymbol, and SChar #57

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Add instances for SNat, SSymbol, and SChar #57

merged 1 commit into from
Oct 24, 2023

Conversation

RyanGlScott
Copy link
Contributor

@RyanGlScott RyanGlScott commented Mar 15, 2023 •
edited
Loading

These were introduced to GHC.TypeNats and GHC.TypeLits in base-4.18.0.0. They did not receive Eq or Ord instances until base-4.19.0.0, however, which are required for the superclasses of EqP and OrdP. To account for this, I introduce a conditional dependency on base-orphans, which backports the Eq and Ord instances to older versions of base.

@RyanGlScott RyanGlScott requested a review from phadej March 15, 2023 11:37
phadej
phadej reviewed Mar 15, 2023
View reviewed changes
Copy link
Collaborator

@phadej phadej left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There's now also EqP and OrdP, if you don't mind adding instance for them, it would be great

@RyanGlScott
Copy link
Contributor Author

SNat and friends don't have Eq or Ord instances, so they wouldn't be able to satisfy the Eq/Ord superclasses of EqP/OrdP.

@phadej
Copy link
Collaborator

phadej commented Mar 15, 2023 •
edited
Loading

@RyanGlScott they really should (even they are trivial).GEq will get these super-class constraints too (by having EqP as superclass)

@RyanGlScott
Copy link
Contributor Author

Yes, I agree. Unfortunately, I overlooked them when proposing haskell/core-libraries-committee#85, and the ship has already sailed for GHC 9.6 additions to base...

@phadej
Copy link
Collaborator

phadej commented Mar 15, 2023 •
edited
Loading

There's future base versions and base-orphans, so the ship is still in sight ;)

But I'd rather not merge this until the missing instance are added, otherwise the adding of superclass to GEq will be blocked (or the instances will need to be removed).

@RyanGlScott
Copy link
Contributor Author

Fair enough. I will try to find time to write a couple follow-up CLC proposals to expand the SNat API. (I thought I was thorough, but there is a lot of ground to cover for that one type!)

@RyanGlScott
Copy link
Contributor Author

Good news: the Eq and Ord instances for SNat et al. just landed upstream in GHC in this commit. I will mark this PR as a draft until there is a new base-orphans release with these instances available.

@RyanGlScott RyanGlScott marked this pull request as draft March 26, 2023 00:33
@RyanGlScott RyanGlScott force-pushed the SNat-SSymbol-SChar-instances branch from 64c829b to 8d45aaa Compare October 11, 2023 11:45
@RyanGlScott
Copy link
Contributor Author

base-orphans-0.9.1 has been released, which backports the Eq and Ord instances for SNat and friends to older versions of base. I've updated this PR to conditionally depend on base-orphans to ensure that these instances are always in scope, which finally allows us to define EqP and OrdP instances.

@RyanGlScott RyanGlScott marked this pull request as ready for review October 11, 2023 11:47
@RyanGlScott RyanGlScott requested a review from phadej October 11, 2023 11:47
phadej
phadej requested changes Oct 12, 2023
View reviewed changes
src/Data/EqP.hs Outdated Show resolved Hide resolved
@RyanGlScott
Add instances for SNat, SSymbol, and SChar
f772966 
These were introduced to `GHC.TypeNats` and `GHC.TypeLits` in `base-4.18.0.0`.
They did not receive `Eq` or `Ord` instances until `base-4.19.0.0`, however,
which are required for the superclasses of `EqP` and `OrdP`. To account for
this, I introduce a conditional dependency on `base-orphans`, which backports
the `Eq` and `Ord` instances to older versions of `base`.
@RyanGlScott RyanGlScott force-pushed the SNat-SSymbol-SChar-instances branch from 8d45aaa to f772966 Compare October 12, 2023 17:58
@phadej phadej self-assigned this Oct 23, 2023
phadej
phadej approved these changes Oct 24, 2023
View reviewed changes
Copy link
Collaborator

@phadej phadej left a comment

Choose a reason for hiding this comment

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

This looks good. Sorry for delay, I missed your comment that you changed this.

@phadej phadej merged commit 44f0e96 into master Oct 24, 2023
8 checks passed
@phadej phadej deleted the SNat-SSymbol-SChar-instances branch October 24, 2023 21:04
@phadej
Copy link
Collaborator

phadej commented Oct 24, 2023

https://hackage.haskell.org/package/some-1.0.6 is on hackage

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

@phadej phadej phadej approved these changes

Assignees

@phadej phadej

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RyanGlScott @phadej