[Typechecker] Allow matching an enum case against an optional enum without '?'

[theblixguy]

This PR lifts a restriction on switching over enum cases when the enum is an Optional. Previously, if the enum was optional, you had to use the ? operator to switch over the cases, otherwise an error diagnostic was emitted telling the user that the case does not exist in the enum.

Now, the compiler will synthesise a OptionalSomePattern to match a case without a trailing ?.

Example:

enum Foo {
  case one
  case two
}

let foo: Foo? = .one

switch foo {
  case .one: print("one") // Ok
  case .two?: print("two") // Ok
  default: print("default")
}

Resolves SR-7799.
Resolves rdar://problem/41494702

[theblixguy]

cc @jrose-apple @xedin @jckarter

[slavapestov]

Is this considered a bug fix and not a language change? I guess it makes sense that the behavior is more consistent with non-enum patterns now.

Can you add a test that it SILGens correctly too?

[slavapestov]

@swift-ci Please test

[slavapestov]

@swift-ci Please test source compatibility

[theblixguy]

@slavapestov I have added a SILGen test.

Seems like the build stopped running on the CI, it says "Jenkins is going to shut down". Nevermind, the test passed.

[CodaFi]

Let me start by saying I am strongly against changes that add implicit conversions into the semantics of pattern matching. I believe this is a language change that needs to go through Swift Evolution, and is not a bug fix. I have articulated this in a reply to the author of the bug report on Swift Evolution in the past.

[CodaFi]

Also, please make sure that this is well-defined and included in the test suite for this patch:

enum Foo {
  case one
  case two
}

let foo: Foo?? = .one

switch foo {
  case .none: // To which `none` does this bind?
  default: print("default")
}

[theblixguy]

Thanks @CodaFi, I have updated the tests. @jckarter said in the same thread that this is actually a bug and it should compile (hence the PR), but if it's decided that this isn't a bug, then I am happy to pitch it and take it through Swift Evolution.

[jckarter]

I consider this a bug fix because it is inconsistent that the implicit conversion in fact works via an expr pattern unless you have a pattern node in the expression. That seems inconsistent to me. As a rule of thumb, a pattern should match values whose expression form looks like the pattern with the _ and let x blanks filled in. I'm not against discussing this in evolution, but it seems like the current half-working state is clearly a bug to me.

[jrose-apple]

I'll say the implementation looks good to me, but it'd be worth re-getting approval from the core team that this should be considered a bug fix, especially in light of @CodaFi's concerns.

[theblixguy]

Tagging core team members: @DougGregor @rjmccall @dabrahams @lattner @airspeedswift @tkremenek

[jckarter]

@swift-ci Please test

[jckarter]

@swift-ci Please test source compatibility

[jckarter]

The Core Team has decided that this is a bug fix.

[jckarter]

@swift-ci Please test

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - e8b5f1e

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - e8b5f1e

[rjmccall]

@swift-ci Please test.

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 407c38e

[rjmccall]

@swift-ci Please test OS X.

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 407c38e

[rjmccall]

Yeesh, that took a long time to kick off.

[theblixguy]

Looks like the tests have passed smoothly 🎉

[jckarter]

Thanks @theblixguy!