Skip to content

Require Applicative (g s) as a superclass of FieldGrammar#11821

Open
Bodigrim wants to merge 1 commit into
masterfrom
require-applicative-for-FieldGrammar
Open

Require Applicative (g s) as a superclass of FieldGrammar#11821
Bodigrim wants to merge 1 commit into
masterfrom
require-applicative-for-FieldGrammar

Conversation

@Bodigrim
Copy link
Copy Markdown
Collaborator

I think we can finally act on the old TODO, cutting a bit of boilerplate.


Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

Thus acting on the old todo, which helps a little bit with boilerplate.
{-# LANGUAGE ConstraintKinds #-}
{-# LANGUAGE FunctionalDependencies #-}
{-# LANGUAGE KindSignatures #-}
{-# LANGUAGE QuantifiedConstraints #-}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need quantified constraints? I just removed these because it doesn't work well with type families in my exact print prototype.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We would not be able to constrain FieldGrammar by forall s. Applicative (g s) without {-# LANGUAGE QuantifiedConstraints #-} enabled. Why does it mess up your type families?

(This PR is just a quality-of-life thing, I'm prepared to abandon it if it gets in the way of more important projects)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My bad, I just tried to apply your changes here to exact print prototype and it worked fine. Last time I had trouble with QuantifiedConstraints was because the quantified constraint was a type family, which GHC can't deal with. In this case s is not a type family.

It's all good for me :)

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.

4 participants