Skip to content

Fix: use Unsatisfiable for banned instances (unrestricted Monoid / Semigroup for linear containers)#494

Merged
aspiwack merged 1 commit intotweag:masterfrom
konn:fix-violation
Jan 27, 2026
Merged

Fix: use Unsatisfiable for banned instances (unrestricted Monoid / Semigroup for linear containers)#494
aspiwack merged 1 commit intotweag:masterfrom
konn:fix-violation

Conversation

@konn
Copy link
Copy Markdown
Contributor

@konn konn commented Dec 30, 2025

Currently, Prelude.Semigroup instance for (linear) HashMap / Set results in a runtime error.
This PR explicitly bans these instances by using Unsatisfiable type constraint family.

@treeowl
Copy link
Copy Markdown
Collaborator

treeowl commented Dec 30, 2025

You have multiple build failures. Unsatisfiable is only available with base-4.19.0.0. Before that, there are some other tricks you can play. Here's a nice version I wrote a while back.

@konn
Copy link
Copy Markdown
Contributor Author

konn commented Dec 30, 2025

@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.

@konn
Copy link
Copy Markdown
Contributor Author

konn commented Dec 30, 2025

Ah, indeed there is already one - `Prelude.Linear.Unsatisfiable. In this time, I just use them.

Copy link
Copy Markdown
Member

@aspiwack aspiwack left a 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?

@treeowl
Copy link
Copy Markdown
Collaborator

treeowl commented Jan 19, 2026

An Unsatisfiable constraint on an instance serves two main purposes:

  1. It documents (in the type error) that the instance is not unintentionally missing. This prevents, e.g., feature requests to have it added.
  2. It ensures that no one wastes (further) time trying to write said instance. It can't be written and someone has checked carefully that that's the case.

@aspiwack
Copy link
Copy Markdown
Member

Fair enough.

Are the messages I suggest clear enough? I haven't spend much time wordsmithing.

@konn
Copy link
Copy Markdown
Contributor Author

konn commented Jan 20, 2026

@treeowl Thank you for clarifying the purpose of Unsatisfiable. Yes, that's exactly my intention to prevent illegal orphan instances.

@aspiwack Thanks! I think it looks good, and committed into the PR.

Unrestricted Semigroup/Monoid instances for containers whose values are
always linear.
Copy link
Copy Markdown
Member

@aspiwack aspiwack left a comment

Choose a reason for hiding this comment

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

I cleaned up the history. Let's merge.

@aspiwack aspiwack enabled auto-merge January 27, 2026 08:53
@aspiwack aspiwack merged commit bcc0a1a into tweag:master Jan 27, 2026
7 checks passed
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.

3 participants