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

typechecker: check exhaustiveness against refined args #411

Closed
wants to merge 1 commit into from

Conversation

berbiche
Copy link
Contributor

@berbiche berbiche commented May 9, 2022

This PR changes the logic in the exhaustiveness check to check for refinability of the refined args instead of the non-refined args.

This was suggested in a different issue here: #339 (comment)
The change resolves an issue I encounter where the type checked for exhaustiveness is accepted but the refined type fails with a badargs in pick_value.

As far as I understand, the refinable3/ function also acts as a gatekeeper for pick_value/2.

edit: Fixes #414

@berbiche berbiche marked this pull request as draft May 10, 2022 14:16
@berbiche berbiche closed this May 10, 2022
@berbiche berbiche reopened this May 17, 2022
@berbiche
Copy link
Contributor Author

One issue with checking refinement against the refined args is that pick_value is called against the refined type instead of the non refined type.

For instance, the refined type of a char is an integer range. The information is more exact but loses meaning for the user (IMO).

@zuiderkwast
Copy link
Collaborator

the refined type of a char is an integer range

Can we turn it into a char range? Or is it normalized here?

IIRC, normalize turns chars into integers, but in some cases (like this) it'd be good to keep them as chars. OTOH in other cases it's good to have a unique representation to pattern match on.

The information is more exact but loses meaning for the user (IMO).

I agree, but maybe we can live with a more obscure message if we get better exactness?

@berbiche
Copy link
Contributor Author

berbiche commented May 18, 2022

the refined type of a char is an integer range

Can we turn it into a char range? Or is it normalized here?

IIRC, normalize turns chars into integers, but in some cases (like this) it'd be good to keep them as chars. OTOH in other cases it's good to have a unique representation to pattern match on.

Yes char() is normalized to an integer range.

expand_builtin_aliases({type, Ann, char, []}) ->
{type, Ann, range, [{integer, Ann, 0}, {integer, Ann, 16#10ffff}]};

I think it could be possible to extend the range type to use chars like type(range, [type(char), type(char)])?

The information is more exact but loses meaning for the user (IMO).

I agree, but maybe we can live with a more obscure message if we get better exactness?

Right, like you've said it's a trade-off between correctness and UX.
I would prefer to be showed a char that isn't matched than a random integer.

@berbiche
Copy link
Contributor Author

Closing my PR as recent changes (and new issues) have invalidated the need for this change.

@berbiche berbiche closed this Sep 12, 2022
@berbiche berbiche deleted the exhaustiveness-refined-args branch September 12, 2022 17:25
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.

exhaustiveness issue alert for wrong parameter
2 participants