Enum generation can make code that causes new Swift 5.1 error message

[thomasvl]

A .proto file with an enum with a lot of cases ends up needing to generate switch statements with a lot of cases. As of Swift 5.1 this can cause the compiler to actually error out.

SwiftProtobuf sees this itself with one of the generated_swift_names_*.swift files:

Tests/SwiftProtobufTests/generated_swift_names_enum_cases.pb.swift:1749:5: error: the compiler is unable to check that this switch is exhaustive in reasonable time
    switch self {
    ^
Tests/SwiftProtobufTests/generated_swift_names_enum_cases.pb.swift:1749:5: note: do you want to add a default clause?
    switch self {
    ^

On the near/short term, we need to do something to tweak what the validation generates to work around this, but…

Longer term, we likely need to see if something can be generated differently/annotate the source in some way to avoid this problem.

[thomasvl]

https://bugs.swift.org/browse/SR-7907 appears to be the upstream bug for this. @allevato is likely going to ping that also.

[thomasvl]

Capturing it since we've been discussing it locally -

• If we add a default all the time, then the compiler does kick out a warning when it is able to safely check the switch in time.
• Adding a default also means some degree of code bloat because if the compiler can't tell everything was covered, it won't realized whatever code we put into the default is never hit and thus won't dead strip it.

[thomasvl]

Also, just switching this test file to proto2 syntax doesn't make it work, there are enough cases even without the one having an associated value to still cause a the issue.

[allevato]

Jordan Rose recommended filing a new bug since the original one was resolved, so let's track https://bugs.swift.org/browse/SR-11533 now.

[thomasvl]

So about the only thing we seem to be able to come up with is do something like:

  var rawValue: Int {
    switch self {
    case .case0: return 0
    case .case1: return 1
    case .case2: return 2
    case .case3: return 3
    case .case4: return 4
    case .case5: return 5
    // ...
    case .caseN: return N
    default: break // nothing
    }
    switch self {
    case .caseN_1: return N+1
    case .caseN_2: return N+2
    case .caseN_3: return N+3
    case .caseN_4: return N+4
    case .caseN_5: return N+5
    // ...
    case .case2N: return 2*N
    default: break // nothing
    }
    // ...
    if case let .UNRECOGNIZED(i) == self { return i }

    // Can't get here, all the cases are listed above.
    // See https://github.com/apple/swift-protobuf/issues/904 for more.
    fatalError()
  }

The UNRECOGNIZED only being there for proto3 syntax, and the hard part being we have to pick some N that will work to avoid this across compiler versions.

The downsides being, this likely generates extra code because it isn't a single switch, the fatalError is going to be generated since this is all because the compiler wasn't able to figure out things were complete in the first case. So anything that goes over the N we pick likely suffers in codegen and maybe performance.

[tbkka]

So anything that goes over the N we pick likely suffers in codegen and maybe performance.

On the other hand, that N is likely to be 100 or more. Such large enums are unusual, and when they do occur, a few additional cases should add no more than a few percent to either size or time.

[thomasvl]

And even that logic was overly complex.

All we need is if over the limit, append a default and then throw the fatalError after it. done. The presence of the default avoid the compiler issue, no reason to use multiple switches, etc.

[thomasvl]

And that doesn't work, if you add the default then you get passed the compiler error, but then the compiler ends up being able to check it and issues a warning about the default not being needed.