swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.58k stars 10.36k forks source link

[SR-5283] Regression: Compiler complains about missing clause when using switch statement #47858

Closed tkrajacic closed 7 years ago

tkrajacic commented 7 years ago
Previous ID SR-5283
Radar None
Original Reporter @tkrajacic
Type Bug
Status Closed
Resolution Done
Environment Version 9.0 beta 2 (9M137d) macOS Sierra 10.12.5 Swift 4 in Swift 3 and 4 modes
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, 4.0Regression | |Assignee | None | |Priority | Medium | md5: 92575e5cb725cc5b7d10a6053bb12459

Issue Description:

With the latest Xcode 9b2 there is a serious regression in the compiler when checking enum exhaustiveness. Sadly I have not been able to reproduce a testcase even though I spend !@#$% 2h on it. I am seriously frustrated now.

What I could find out (but haven't been able to reproduce in a clean new minimal project) is that one of my frameworks has a Property.Value enum with cases:

extension Property {

    public enum Value {
        case intValue(Int)
        case boolValue(Bool)
        case stringValue(String)
        case decimalValue(NSDecimalNumber)
        case rating(Int)
        case dateValue(date: Date, timezone: TimeZone)
        case money(amount: NSDecimalNumber, currency: ISO4217.Currency) // This one !@#s up
        case entity(identifier: String, type: Property.Value.ReferrableEntity)
        case invalid
    }
}

And later in the same file the compiler mistakenly now tells me that my switch is not exhaustive:

extension Property.Value: RawProducable {
    var rawTypeValue: RawTypeRepresentation {
        switch self {
        case .intValue:     return Property.Value.IntegerRep
        case .boolValue:    return Property.Value.BoolRep
        case .stringValue:  return Property.Value.StringRep
        case .decimalValue: return Property.Value.DecimalRep
        case .rating:       return Property.Value.RatingRep
        case .dateValue:    return Property.Value.DateRep
        case .money:        return Property.Value.MoneyRep
        case .entity:       return Property.Value.EntityRep
        case .invalid:      return Property.Value.InvalidRep
        }
    }
}

It suggests a fix to add a default case but after that the compiler issues a warning that the default will never be used. Interestingly after typing an emtpy switch on self, I get a fixit that suggests only the default case (and I thought now the compiler usually suggests to add all cases).

The problematic case `.money` is the reason for that whole mess – more specifically the `ISO4217.Currency` enum that is defined in another framework. Replacing this with a String value for example "fixes" the compiler bug (but is of course not an option).

Needless to say this all worked well in Swift 3.x and up until Xcode 9 b2.
Everything is compiled using Swift 4 (also all the frameworks)

I am really exhausted and need help figuring this out.
I have a lot of work to do and can't spend my day distilling a huge project into test cases.
Of course for now I can add the default case, but then the compiler still has a bug and no one is served.

swift-ci commented 7 years ago

Comment by Ben A (JIRA)

Here's a minimal test case that reproduces this behavior in Swift 4:

enum E<T, U: Error> {
    case a(T)
    case b(U)
    case c
    case d
}

// Borrowed from https://github.com/rhodgkins/SwiftHTTPStatusCodes/blob/master/Sources/HTTPStatusCodes.swift
enum HTTPStatusCode: Int {
    case `continue` = 100
    case switchingProtocols = 101
    case processing = 102

    // Success
    case ok = 200
    case created = 201
    case accepted = 202
    case nonAuthoritativeInformation = 203
    case noContent = 204
    case resetContent = 205
    case partialContent = 206
    case multiStatus = 207
    case alreadyReported = 208
    case imUsed = 226

    // Redirections
    case multipleChoices = 300
    case movedPermanently = 301
    case found = 302
    case seeOther = 303
    case notModified = 304
    case useProxy = 305
    case switchProxy = 306
    case temporaryRedirect = 307
    case permanentRedirect = 308

    // Client Errors
    case badRequest = 400
    case unauthorized = 401
    case paymentRequired = 402
    case forbidden = 403
    case notFound = 404
    case methodNotAllowed = 405
    case notAcceptable = 406
    case proxyAuthenticationRequired = 407
    case requestTimeout = 408
    case conflict = 409
    case gone = 410
    case lengthRequired = 411
    case preconditionFailed = 412
    case requestEntityTooLarge = 413
    case requestURITooLong = 414
    case unsupportedMediaType = 415
    case requestedRangeNotSatisfiable = 416
    case expectationFailed = 417
    case imATeapot = 418
    case authenticationTimeout = 419
    case unprocessableEntity = 422
    case locked = 423
    case failedDependency = 424
    case upgradeRequired = 426
    case preconditionRequired = 428
    case tooManyRequests = 429
    case requestHeaderFieldsTooLarge = 431
    case loginTimeout = 440
    case noResponse = 444
    case retryWith = 449
    case unavailableForLegalReasons = 451
    case requestHeaderTooLarge = 494
    case certError = 495
    case noCert = 496
    case httpToHTTPS = 497
    case tokenExpired = 498
    case clientClosedRequest = 499

    // Server Errors
    case internalServerError = 500
    case notImplemented = 501
    case badGateway = 502
    case serviceUnavailable = 503
    case gatewayTimeout = 504
    case httpVersionNotSupported = 505
    case variantAlsoNegotiates = 506
    case insufficientStorage = 507
    case loopDetected = 508
    case bandwidthLimitExceeded = 509
    case notExtended = 510
    case networkAuthenticationRequired = 511
    case networkTimeoutError = 599
}

enum SomeError: Error {
    case httpError(statusCode: HTTPStatusCode, info: String?)
}

func foo(e: E<Void, SomeError>) {
    switch e {
        case .a: break
        case .c: break
        case .b, .d: break
    }
}

This builds with no errors in Swift 3.1, but it fails while running either of the commands in Swift 4:
swift -swift-version 3 file.swift
swift file.swift

Error:

test.swift:98:5: error: switch must be exhaustive
switch e {
^
test.swift:98:5: note: do you want to add a default clause?
switch e {
^

swift-ci commented 7 years ago

Comment by Ben A (JIRA)

This builds fine in Xcode 9b1 but errors in Xcode 9b2.

swift-ci commented 7 years ago

Comment by Ben A (JIRA)

In my case, you can get it to build if you remove the "success" (i.e. 2** values) codes/cases from the enum, so it's really unclear to me what's going on.

tkrajacic commented 7 years ago

Awesome Ben. Thx for your testcase!

belkadan commented 7 years ago

CodaFi (JIRA User), did we really have different behavior between beta 1 and beta 2?

Regardless, Ben's minimal example passes for me on master, and this sounds a lot like one of the things you fixed recently.

CodaFi commented 7 years ago

We merged a bad heuristic in beta 2 and corrected it for beta 3 here. So sorry for the breakage.

CodaFi commented 7 years ago

The workaround for this case is to explicitly write out the previously-elided any-patterns.

func foo(e: E<Void, SomeError>) {
    switch e {
        case .a(_): break
        case .c: break
        case .b(_), .d: break
    }
}

Before that patch we weren't actually representing the elision in the AST, so the space engine just assumed that the total number of spaces you covered was far lower than what was actually there.

swift-ci commented 7 years ago

Comment by Ben A (JIRA)

Ah okay. Thanks for the workaround CodaFi (JIRA User)!

tkrajacic commented 7 years ago

I can confirm that it is fixed in the latest Xcode 9b3 release