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

Add subclasses to enforce the type class laws #126

Merged
merged 1 commit into from
Oct 15, 2020

Conversation

ursi
Copy link
Contributor

@ursi ursi commented Jul 17, 2020

No description provided.

@hdgarrood
Copy link
Contributor

I’m hesitant about this one, because we are now carrying extra dictionaries around which we weren’t before. At the very least this should allow us to relax some constraints elsewhere, in instances for these classes.

@hdgarrood
Copy link
Contributor

Sorry, that’s not quite right - instances will likely still need the same constraints. I was thinking that there may now be redundant constraints on functions which use MonadTell or MonadWriter generically, but we only have two of those (listens and censor) and neither of them have an additional Semigroup or Monoid constraint. So 👍 from me. Thanks!

@hdgarrood hdgarrood merged commit aa1fba3 into purescript:master Oct 15, 2020
@ursi
Copy link
Contributor Author

ursi commented Oct 15, 2020

oopsie daisy, I added superclasses, not subclasses. I think I was confused when I made this PR (<= while being an arrow, can also be interpreted as less-than-or-equal-to)

Edit: actually I don't even know what to call it, since it's a type class constraint on one of the two types in the type class.

@hdgarrood
Copy link
Contributor

Haha, so you did! I didn't realise until just now either.

@hdgarrood
Copy link
Contributor

I think superclasses is still accurate - it's a relationship between the classes themselves, not the type parameters.

@ursi
Copy link
Contributor Author

ursi commented Oct 15, 2020

I'm willing to redo the commit and submit another PR if you'd like it to be named appropriately.

@hdgarrood
Copy link
Contributor

I wouldn't worry about it. We'd need to rewrite the history, which is a much bigger no-no in my mind.

thomashoneyman added a commit that referenced this pull request Jan 20, 2021
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