Skip to content

Expand error message for UnusableDeclaration#3088

Merged
LiamGoodacre merged 1 commit intopurescript:masterfrom
i-am-tom:master
Sep 24, 2017
Merged

Expand error message for UnusableDeclaration#3088
LiamGoodacre merged 1 commit intopurescript:masterfrom
i-am-tom:master

Conversation

@i-am-tom
Copy link
Copy Markdown

Previously, the error message made no mention of a solution to the error. This update adds possible solutions to the error message in the form of the remaining covering sets. In other words, the remaining variables that need to be determined in order to satisfy the typechecker.

I think a couple of these changes look a bit ugly, so please do leave opinions :)

@LiamGoodacre
Copy link
Copy Markdown
Member

Thanks. Please add yourself to the contributors file.

Previously, the error message made no mention of a solution to the
error. This update adds possible solutions to the error message in
the form of the remaining covering sets. In other words, the remaining
variables that need to be determined in order to satisfy the
typechecker.
@i-am-tom
Copy link
Copy Markdown
Author

@LiamGoodacre Done! Also added some more TLC to the error messages, so they're hopefully a bit easier to read in the simpler case.

@garyb
Copy link
Copy Markdown
Member

garyb commented Sep 24, 2017

Still no mention of fundeps being the determinant? 😉

@LiamGoodacre
Copy link
Copy Markdown
Member

Fundeps aren't necessarily involved here. Consider:

class Foo t where
  foo :: String

@LiamGoodacre
Copy link
Copy Markdown
Member

LiamGoodacre commented Sep 24, 2017

I mean you could argue that is like | t -> t - but it's not explicit.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Sep 24, 2017

Or | -> t should solve it too.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Sep 24, 2017

Strangely it doesn't though.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Sep 24, 2017

👍 Looks good to me. I think we can add an extended explanation of fundeps in the docs repo.

@LiamGoodacre
Copy link
Copy Markdown
Member

Strangely it doesn't though.

@paf31 hmm, sounds like there's an issue with this edge case when we build the covering sets.

@LiamGoodacre LiamGoodacre merged commit 15b5827 into purescript:master Sep 24, 2017
@i-am-tom
Copy link
Copy Markdown
Author

@paf31 It's on my list of docs to expand :) PR to follow in the next couple days!

paf31 pushed a commit that referenced this pull request Nov 8, 2017
Previously, the error message made no mention of a solution to the
error. This update adds possible solutions to the error message in
the form of the remaining covering sets. In other words, the remaining
variables that need to be determined in order to satisfy the
typechecker.
jutaro pushed a commit to mgmeier/purescript that referenced this pull request Mar 12, 2018
Previously, the error message made no mention of a solution to the
error. This update adds possible solutions to the error message in
the form of the remaining covering sets. In other words, the remaining
variables that need to be determined in order to satisfy the
typechecker.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants