Skip to content
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

[SR-7001] Error considered to conform to CustomStringConvertible in switch #49549

Open
swift-ci opened this issue Feb 14, 2018 · 5 comments
Open
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-7001
Radar rdar://problem/37573072
Original Reporter deanWombourne (JIRA User)
Type Bug
Environment

> swift --version

Apple Swift version 4.0.3 (swiftlang-900.0.74.1 clang-900.0.39.2)

Target: x86_64-apple-macosx10.9

(I'm in Xcode Version 9.2 (9C40b) but I don't think that's relevant)

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, Miscompile
Assignee None
Priority Medium

md5: aa483efddb27c4b86b69f8a69c2f5c96

Issue Description:

If I create this enum

public enum Result<T> {
   case success(T)
   case failure(Error)
 }

and implement CustomStringConvertible like this:

extension Result: CustomStringConvertible {  
  public var description: String {
    switch self {
    case .success(let value):
      return "Result.success(\(value))"
    case .failure(let error as CustomStringConvertible):
      return "Result.failure(\(error.description))"
    }
  }
}   

The compiler considers this to be exhaustive.

This is not the case i.e.

struct MyError: Error { }

Result.failure(MyError()).description

doesn't match any of the cases in the switch. This causes, erm, unpredictable behaviour.

Also, if I add a case to cover Error types which don't implement CustomStringConvertible, the compiles warns me that it is unnecessary. i.e. adding

 case .failure(let error):
      return "Result.failure(\(error))"

causes "Case is already handled by previous patterns; consider removing it"

@hamishknight
Copy link
Collaborator

Note that you need to {{import Foundation}} to reproduce this; looks like the switch should be bridging to {{NSError}} but isn't.

@belkadan
Copy link
Contributor

@swift-ci create

@rudkx
Copy link
Member

rudkx commented Feb 17, 2018

CodaFi (JIRA User), any ideas on this?

@CodaFi
Copy link
Member

CodaFi commented Feb 19, 2018

This would have to be a problem in TypeCheckPattern. Space Engine relies on the type checker telling it that cast patterns are well-formed.

@hamishknight
Copy link
Collaborator

Unless I'm mistaken, I think the true problem lies in the runtime; after checking to see if the value conforms to CustomStringConvertible, it should then check its bridged type for conformance (in a way, it's a similar issue to https://bugs.swift.org/browse/SR-3871 where the bridged type isn't checked for protocol conformance).

The reason this particular bug isn't encountered with as? and as! (though it is with is), for example:

extension Result: CustomStringConvertible {
  public var description: String {
    switch self {
    case .success(let value):
      return "Result.success(\(value))"
    case .failure(let error):
      print(error is CustomStringConvertible) // false...
      return "Result.failure(\((error as! CustomStringConvertible).description))"
    }
  }
}

struct EE : Error {}
print(Result<Void>.failure(EE()))
// Result.failure(Error Domain=EE Code=1 "(null)")

is because such bridging coercions are caught and "filled in" at compile time rather than relying on the runtime. I don't believe we can do the same thing in pattern matching though, so I think the runtime is the place to fix this.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

5 participants