-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Check exhaustiveness for int, str, ... -typed alts #1720
Comments
I'm not 100% sure if this is desirable. This seems like a case where non-exhaustive by default could be the right thing. Adding "rfc" tag for that reason :) |
Fair enough. My opinion is that if we can prevent an uncaught exception from happening through static checks, we should. As with the other kind of alt, if the programmer's intent is that a certain case should never happen, they should make that explicit. If we do make them non-exhaustive, I would advocate using a different keyword (as painful as that is) like "alt?" or something, so as not to give the impression that alts on non-algebraic data give the same static guarantee as the other sort. |
I'd (still) prefer a single default (exhaustive) with the ability to explicitly mark individual alts as non-exhaustive. |
Non-obvious. Consider this my expression of "please don't merge yet, we need to discuss more". Might be another argument for playing with the |
There's nothing to merge yet anyway :-) I'm not really in a rush to do anything about this, since this form of alt isn't as common as alts on enums. |
I think we can basically just treat this like OCaml, where int and str are always considered nonexhaustive and you have to insert a _. (For str this is always true, for int it might as well be—nobody will write a 4-billion-case alt.) |
@pcwalton This is what seemed right to me, too. |
shrug I don't have a strong opinion. A single default is probably best. Least surprising. |
The patch I just pushed only handles exhaustiveness for alts on enum-typed data. alts on ints or strings or such can still be non-exhaustive. Fix this. (Obviously this would require inserting an '_' case most of the time.)
The text was updated successfully, but these errors were encountered: