-
Notifications
You must be signed in to change notification settings - Fork 41
Fix: use Unsatisfiable for banned instances (unrestricted Monoid / Semigroup for linear containers) #494
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
base: master
Are you sure you want to change the base?
Conversation
|
You have multiple build failures. |
|
@treeowl Thank you for pointing it out! Then I will introduce compat module before/after GHC 9.8 and put your code (with naming tweaking) for older versions. |
|
Ah, indeed there is already one - `Prelude.Linear.Unsatisfiable. In this time, I just use them. |
aspiwack
left a comment
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.
I'm not too sure about this. The message isn't suppose to ever reach the user, if the message surfaces it would be a bug in the library. Have you experienced this error message? (if so, please report a bug 🥺)
I don't actually remember why we have these instances to begin with. My best guess is that at some point, we'd decided that the Prelude's Semigroup class was a superclass to the linear Semigroup class. And so we'd absolutely needed a Prelude.Semigroup instance for Hashmaps if they were to have a linear Semigroup instance.
But maybe we don't need these instances. How about simply deleting them instead?
|
An
|
|
Fair enough. Are the messages I suggest clear enough? I haven't spend much time wordsmithing. |
Co-authored-by: Arnaud Spiwack <arnaud@spiwack.net>
Currently,
Prelude.Semigroupinstance for (linear) HashMap / Set results in a runtime error.This PR explicitly bans these instances by using
Unsatisfiabletype constraint family.