-
Notifications
You must be signed in to change notification settings - Fork 13
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
Draft: Redefines the EqF type class to be heterogeneous. #132
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -214,6 +214,14 @@ instance TestEquality f => TestEquality (List f) where | |
pure Refl | ||
testEquality _ _ = Nothing | ||
|
||
instance EqF f => EqF (List f) where | ||
eqF Nil Nil = Just Refl | ||
eqF (xh :< xl) (yh :< yl) = do | ||
Refl <- eqF xh yh | ||
Refl <- eqF xl yl | ||
pure Refl | ||
eqF _ _ = Nothing | ||
|
||
instance OrdF f => OrdF (List f) where | ||
compareF Nil Nil = EQF | ||
compareF Nil _ = LTF | ||
|
@@ -267,10 +275,10 @@ deriving instance Show (Index l x) | |
|
||
instance ShowF (Index l) | ||
|
||
instance TestEquality (Index l) where | ||
testEquality IndexHere IndexHere = Just Refl | ||
testEquality (IndexThere x) (IndexThere y) = testEquality x y | ||
testEquality _ _ = Nothing | ||
Comment on lines
-270
to
-273
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I presume that this instance was removed because it does not obey the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, and I actually meant to put this back in to allow just such a gradual change. Initially I had actually removed the instances in Do you know if there is a way to deprecate a type class instance and have GHC give a warning that they should use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not aware of a way to attach I am curious exactly how much churn this PR would entail. Even if we keep around all of the existing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, this would entail a lot of churn -- not only will we uncover missing But yeah, the solution should just be to copy-paste instances. I made this PR in discussion with RobD, as a way to start the conversation about whether this change is worth it. I think it is worth it, personally -- insofar as |
||
instance EqF (Index l) where | ||
eqF IndexHere IndexHere = Just Refl | ||
eqF (IndexThere x) (IndexThere y) = eqF x y | ||
eqF _ _ = Nothing | ||
|
||
instance OrdF (Index l) where | ||
compareF IndexHere IndexHere = EQF | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be helpful to provide an example of code that is not a legal
TestEquality
instance but is a legalEqF
instance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call.