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

Remove the check for union types on match error #1630

Merged
merged 2 commits into from
Mar 4, 2017
Merged

Conversation

sylvanc
Copy link
Contributor

@sylvanc sylvanc commented Mar 3, 2017

A nice attempt to produce a much friendlier error message when a union
type pattern is involved and a match expression doesn't type check is
sadly removed in this commit.

Unfortunately, it assumed union types would be composed of nominal types
(not true), and that the problem with the match was a union type with
varying capabilities (this is actually allowed).

A nicer error message would indeed be useful here. The right approach
would be to emit errors in matchtype.c when they are needed, i.e. on
DENY.

A nice attempt to produce a much friendlier error message when a union
type pattern is involved and a match expression doesn't type check is
sadly removed in this commit.

Unfortunately, it assumed union types would be composed of nominal types
(not true), and that the problem with the match was a union type with
varying capabilities (this is actually allowed).

A nicer error message would indeed be useful here. The right approach
would be to emit errors in `matchtype.c` when they are needed, i.e. on
DENY.
@sylvanc sylvanc added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Mar 3, 2017
@jemc
Copy link
Member

jemc commented Mar 3, 2017

@sylvanc, can we use a more explanatory message than "this capture violates capabilities"?

In #1545 we discussed using something like "this capture type has a mismatch in reference capabilities, which can't be differentiated at runtime", though I'm still open to improvements on that.

@sylvanc
Copy link
Contributor Author

sylvanc commented Mar 4, 2017

We can, but it should be done as part of moving the error generation into matchtype.c, to get more accurate messages. This is just a "rollback" PR.

@sylvanc sylvanc merged commit 925cf63 into master Mar 4, 2017
ponylang-main added a commit that referenced this pull request Mar 4, 2017
@SeanTAllen SeanTAllen deleted the remove-union-check branch April 6, 2017 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants