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

Improve error message for impossible JoinKinds constraint #424

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

adamgundry
Copy link
Member

Fixes #423.

WARNING: typeclass Prolog ahead.

@adamgundry adamgundry requested a review from arybczak June 8, 2021 20:05
@adamgundry adamgundry force-pushed the 423-joinkinds-errors branch from b128e9e to 8cf13be Compare June 8, 2021 21:15
@arybczak
Copy link
Collaborator

arybczak commented Jun 9, 2021

Thanks. Do we want to drop support for GHC 8.2 and 8.4 (as this introduces UndecidableSuperClasses)? I personally don't mind.

EDIT: Never mind, for some reason I thought that this extension was introduced in GHC 8.6.

Also, doctests fail, presumably due to a missing import.

-- @is_concrete@ will be unified with @False@.
--
class IsOpticKind (m :: OpticKind) (is_concrete :: Bool) | m -> is_concrete
instance is_concrete ~ 'True => IsOpticKind An_AffineFold is_concrete
Copy link
Contributor

Choose a reason for hiding this comment

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

What will happen if a kind is forgotten, we get a "wrong" custom type error?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was a bit dissatisfied with the need to list all the optic kinds explicitly, but I couldn't immediately see how to avoid it. Having thought a bit more I've got a version that doesn't need this.

@adamgundry adamgundry force-pushed the 423-joinkinds-errors branch from 8cf13be to f7704fb Compare June 10, 2021 18:27
This is less vulnerable to us forgetting to extend the definition if
we add a new optic kind.
@adamgundry adamgundry force-pushed the 423-joinkinds-errors branch from cffcdf0 to 28feead Compare June 10, 2021 19:04
@adamgundry
Copy link
Member Author

I was playing around with this branch and noticed this:

> view @A_Getter (folded % _) [Just "abc", Nothing, Just "def"]

<interactive>:3:17: error:
    • A_Fold composed with l0 produces p0,
      but the context expects A_Getter

We should be able to do better here, by checking if more parameters are concrete.

On the other hand, we're a bit stuck if the context doesn't force a particular optic kind (cf. #308):

> view (folded % _) [Just "abc", Nothing, Just "def"]

<interactive>:4:1: error:
    • Overlapping instances for Is k0 A_Getter
        arising from a use of ‘view’
...
    • Overlapping instances for JoinKinds A_Fold l0 k0
        arising from a use of ‘%’

@arybczak
Copy link
Collaborator

arybczak commented Jul 8, 2021

What's the status of this PR?

@adamgundry
Copy link
Member Author

I've been away on holiday so apologies for the radio silence. I'm still planning to polish this a bit, but it may be a little while before I can get back to it.

@arybczak
Copy link
Collaborator

arybczak commented Dec 6, 2021

@adamgundry I posted an alternative solution in #439, please have a look if you have a moment. It's not as fancy, but should be more predictable :)

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.

Misleading error message from JoinKinds
3 participants