Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Remove alt check #3101

Closed
brson opened this Issue · 14 comments

7 participants

@brson
Owner

This is an unnecessary corner of the language, the only remaining use of the check keyword. I know I have misexplained the difference between alt and alt check before.

alt check is a bit of an anti-pattern because it:

  • Indicates that enums aren't factored correctly. With case classes we should have more control to prevent this kind of problem.
  • Encourages non-exhaustive matches which are only discovered at runtime.
  • Doesn't provide any documentation about why other cases don't matter.

Which is clearer?

alt check foo {
    bar => baz
}
alt foo {
    bar => baz,
    _ => fail "this shouldn't happen because ..."
}

The first requires explaination. The second says exactly what the fallthrough behavior is, occurs syntactically where the fallthrough behavior occurs, and is more natural to write (once you've written all the cases you intend to cover you then write the fallthrough case instead of going back to the top of the structure and changing it).

@brson
Owner

There's also no equivalent syntax for _ => { /* fallthrough */ } which is just as common a pattern.

@bblum

Related #1679.

I strongly favour this change. I argued with @gwillen about this a while ago; my stance was that the language should make it difficult rather than easy to do this sort of thing. I want a language that forces me to write fail "reason" when I've programmed in an anti-pattern.

I weakly favour match partial showing up as sugar for the fallthrough case.

@gwillen

@bblum I think I withdraw my defense of alt check. _ => fail is not really that much harder to write, and makes you explain yourself better.

@lkuper

+1.

@catamorphism

+MAXINT

@catamorphism

I mentioned this to @brson already, but at least a dozen times I've either run into bugs that resulted from an alt check failing, or made an alt check fail as part of changing the compiler. In every one of these cases, it would have been better for programmer morale to see an informative error message. This also means the thesis that we will only use alt check when we really know the match will never fail is simply false.

We've tried having alt check in the language and it's failed. Exhaustive matches for 2012.

@pcwalton
Owner

+1 on this change.

@eholk

+1

@catamorphism

In the meeting we decided we had consensus to remove this. I volunteer to do so. (Graydon points out that with the macro system, it's easy for someone to write their own alt check macro.)

@catamorphism catamorphism was assigned
@catamorphism

(I might try diving into macros myself to write a "do nothing in all omitted cases" macro and a "fail with a given message in all omitted cases" macro. How hard can it be?)

@eholk

The macro shouldn't be too hard, although you may have to duplicate most of the pattern syntax yourself.

See the follow! macro at https://github.com/eholk/rust/blob/pipes/src/test/run-pass/pipe-bank-proto.rs and the select! macro at https://github.com/eholk/rust/blob/pipes/src/test/run-pass/pipe-select-macro.rs for an example of something that's probably close to what you want.

You might be able to get away with just using token trees rather than duplicating the pattern syntax too.

@catamorphism

I didn't end up doing the macros (could be added later), but otherwise, this is done: 5e22fb9

@bblum

yessss

@lkuper

Hooray!

@catamorphism catamorphism was unassigned by brson
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.