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

Correct logic when to orphan Hashable ByteArray #2460

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

DigitalBrains1
Copy link
Member

@DigitalBrains1 DigitalBrains1 commented Apr 28, 2023

PR #2448 and #2449 were incomplete, missing some cases where we still needed to create an orphan instance for Hashable ByteArray ourselves. This should get it right, including for when we will support GHC 9.4 (which is guarded by the expression MIN_VERSION_base(4,17,0)).

Note that the Package Versioning Policy has something to say about defining orphan instances like this:

Client defines orphan instance. If a package defines an orphan instance, it MUST depend on the minor version of the packages that define the data type and the type class to be backwards compatible. For example, build-depends: mypkg >= 2.1.1 && < 2.1.2.

I think we used to violate this rule, but not any longer :-). Because we now switch off the orphan instance generation starting at a certain version of the dependencies, so we can freely depend in the usual only-major fashion on them again.

Why to be backwards compatible though? The problem here was forwards compatibility, it seems to go both ways. Maybe a simple case of phrasing, or you're supposed to switch your view point, it's always backwards from one side ;-).

Fixes #2453

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

PR #2448 and #2449 were incomplete, missing some cases where we still
needed to create an orphan instance for Hashable ByteArray ourselves.
This should get it right, including for when we will support GHC 9.4
(which is guarded by the expression `MIN_VERSION_base(4,17,0)`).
@DigitalBrains1
Copy link
Member Author

I will add

Fixes #2453

to the commit msg when I squash merge. Forgot that.

@martijnbastiaan
Copy link
Member

LGTM, I'm running the script I posted earlier to verify that this works.

@DigitalBrains1
Copy link
Member Author

If it gives a different result on clash-compiler than it does on test-hashable-primitive I will eat my hat! Not right away, left it at home.

Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

LGTM

OK:   ghc-9.2.7, hashable-1.4.2.0, primitive-0.7.4.0
OK:   ghc-9.2.7, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-9.2.7, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.2.7, hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-9.2.7, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-9.2.7, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-9.0.2, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-9.0.2, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-9.0.2, hashable-1.4.2.0, primitive-0.7.4.0
OK:   ghc-9.0.2, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-9.0.2, hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-9.0.2, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.2.0, primitive-0.7.4.0
OK:   ghc-8.10.7, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.10.7, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.10.7, hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.8.4, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.8.4, hashable-1.4.2.0, primitive-0.7.4.0
OK:   ghc-8.8.4, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.8.4, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.8.4, hashable-1.4.0.2, primitive-0.7.4.0
OK:   ghc-8.8.4, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.6.5, hashable-1.4.1.0, primitive-0.8.0.0
OK:   ghc-8.6.5, hashable-1.4.1.0, primitive-0.7.4.0
OK:   ghc-8.6.5, hashable-1.4.2.0, primitive-0.7.4.0
OK:   ghc-8.6.5, hashable-1.4.2.0, primitive-0.8.0.0
OK:   ghc-8.6.5, hashable-1.4.0.2, primitive-0.8.0.0
OK:   ghc-8.6.5, hashable-1.4.0.2, primitive-0.7.4.0

@DigitalBrains1 DigitalBrains1 merged commit c56131d into master Apr 28, 2023
10 checks passed
@DigitalBrains1 DigitalBrains1 deleted the fix-when-orphan-hash-bytearray branch April 28, 2023 14:26
DigitalBrains1 added a commit that referenced this pull request Jun 2, 2023
This is a backport of a combination of PR's #2448, #2449 and #2460.

For some combinations of `hashable` and `primitive`, there is already a
`Hashable ByteArray` instance and we should not create one.
DigitalBrains1 added a commit that referenced this pull request Jun 2, 2023
This is a backport of a combination of PR's #2448, #2449 and #2460.

For some combinations of `hashable` and `primitive`, there is already a
`Hashable ByteArray` instance and we should not create one.
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.

Missing Hashable instance for clash-lib
2 participants