Provide CustomNSError default implementation #1134

zayass · 2017-07-26T12:08:31Z

Pull default CustomNSError implementation from https://github.com/apple/swift/blob/master/stdlib/public/SDK/Foundation/NSError.swift

ianpartridge · 2017-07-26T14:17:26Z

Can we add some tests in this area?

zayass · 2017-07-26T19:16:24Z

@ianpartridge I add some tests but have no environment to properly run it. Can you trigger CI build?

xwu · 2017-07-27T04:27:12Z

Foundation/NSError.swift

+    // conflict resolution
+	var _code: Int {
+		return errorCode
+	}


Can you fix the indenting on these lines? They should match the rest of the file by using four spaces instead of tabs.

xwu · 2017-07-27T04:27:20Z

Foundation/NSError.swift

+        return numericCast(self.rawValue)
+    }
+
+    // conflict resolution


What does this comment mean?

this method fixes compilation issue which @ianpartridge posted

As I understand this two extensions
https://github.com/apple/swift-corelibs-foundation/blob/master/Foundation/NSError.swift#L282
https://github.com/apple/swift/blob/master/stdlib/public/core/ErrorType.swift#L220
conflict with each other because no one is more specific

This is more specific so wins them all and "resolve conflict"

xwu · 2017-07-27T04:27:29Z

Foundation/NSError.swift

+    // conflict resolution
+	var _code: Int {
+		return errorCode
+	}


ianpartridge · 2017-07-27T08:24:23Z

@swift-ci please test

ianpartridge · 2017-07-27T09:11:05Z

@zayass

TestFoundation/TestNSError.swift:75:14: error: type 'SwiftError' does not conform to protocol 'Error'
        enum SwiftError : Int, Error, CustomNSError {
             ^
Swift.Error:101:16: note: multiple matching properties named '_code' with type 'Int'
    public var _code: Int { get }
               ^
Swift.Error:2:16: note: candidate exactly matches
    public var _code: Int { get }
               ^
Swift.Error:2:16: note: candidate exactly matches
    public var _code: Int { get }
               ^
Foundation.Error:5:16: note: candidate exactly matches
    public var _code: Int { get }
               ^
TestFoundation/TestNSError.swift:85:14: error: type 'SwiftError' does not conform to protocol 'Error'
        enum SwiftError : UInt, Error, CustomNSError {
             ^
Swift.Error:101:16: note: multiple matching properties named '_code' with type 'Int'
    public var _code: Int { get }
               ^
Swift.Error:2:16: note: candidate exactly matches
    public var _code: Int { get }
               ^
Swift.Error:2:16: note: candidate exactly matches
    public var _code: Int { get }
               ^
Foundation.Error:5:16: note: candidate exactly matches
    public var _code: Int { get }
               ^

zayass · 2017-07-27T12:57:17Z

@ianpartridge fix it. Just forgot public for _code extesnion

ianpartridge · 2017-07-27T13:01:13Z

@swift-ci please test

zayass

Found better solution in original https://github.com/apple/swift/blob/master/stdlib/public/SDK/Foundation/NSError.swift

zayass · 2017-07-27T12:55:07Z

Foundation/NSError.swift

+        return numericCast(self.rawValue)
+    }
+
+    // conflict resolution


this method fixes compilation issue which @ianpartridge posted

As I understand this two extensions
https://github.com/apple/swift-corelibs-foundation/blob/master/Foundation/NSError.swift#L282
https://github.com/apple/swift/blob/master/stdlib/public/core/ErrorType.swift#L220
conflict with each other because no one is more specific

This is more specific so wins them all and "resolve conflict"

ianpartridge · 2017-07-27T15:21:08Z

@swift-ci please test

alblue · 2017-07-28T13:57:57Z

Looks good to me. WDYT @ianpartridge ?

ianpartridge · 2017-07-28T14:06:47Z

TestFoundation/TestNSError.swift

+
+    func test_CustomNSError_userInfo() {
+        let userInfo = SwiftCustomNSError().errorUserInfo
+            XCTAssertTrue(userInfo.isEmpty)


Can you fix the indentation of this line?

ianpartridge · 2017-07-28T14:07:48Z

TestFoundation/TestNSError.swift

+    func test_CustomNSError_errorCodeRawInt() {
+        enum SwiftError : Int, Error, CustomNSError {
+            case minusOne  = -1
+            case fourtyTwo = 42


Spelling: fortyTwo 🙂

ianpartridge · 2017-07-28T14:08:03Z

TestFoundation/TestNSError.swift

+
+    func test_CustomNSError_errorCodeRawUInt() {
+        enum SwiftError : UInt, Error, CustomNSError {
+            case fourtyTwo = 42


Spelling: fortyTwo 🙂

ianpartridge · 2017-07-28T14:09:38Z

Minor tweaks but otherwise this looks good to me 👍

Pull default CustomNSError implementation from https://github.com/apple/swift/blob/master/stdlib/public/SDK/Foundation/NSError.swift

zayass · 2017-07-28T16:20:15Z

@ianpartridge fixed

ianpartridge · 2017-07-28T21:36:49Z

@swift-ci please test and merge

ianpartridge · 2017-07-28T22:06:45Z

Thanks @zayass 👍

zayass · 2017-07-31T10:51:09Z

May be I can cherry pick this to 3.1 branch. or it is closed?

ianpartridge · 2017-07-31T11:02:13Z

There isn't going to be another release from the 3.1 branch, I'm afraid.

xwu reviewed Jul 27, 2017

View reviewed changes

zayass force-pushed the patch-4 branch from 675c2a4 to d065739 Compare July 27, 2017 13:54

zayass commented Jul 27, 2017

View reviewed changes

ianpartridge reviewed Jul 28, 2017

View reviewed changes

zayass force-pushed the patch-4 branch from d065739 to 9bbf96d Compare July 28, 2017 16:17

Provide CustomNSError default implementation

511aabc

Pull default CustomNSError implementation from https://github.com/apple/swift/blob/master/stdlib/public/SDK/Foundation/NSError.swift

zayass force-pushed the patch-4 branch from 9bbf96d to 511aabc Compare July 28, 2017 16:19

swift-ci merged commit 7d4da50 into swiftlang:master Jul 28, 2017

Provide CustomNSError default implementation #1134

Provide CustomNSError default implementation #1134

Uh oh!

Conversation

zayass commented Jul 26, 2017

Uh oh!

ianpartridge commented Jul 26, 2017

Uh oh!

zayass commented Jul 26, 2017

Uh oh!

xwu Jul 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xwu Jul 27, 2017

Choose a reason for hiding this comment

Uh oh!

zayass Jul 27, 2017

Choose a reason for hiding this comment

Uh oh!

xwu Jul 27, 2017

Choose a reason for hiding this comment

Uh oh!

ianpartridge commented Jul 27, 2017

Uh oh!

ianpartridge commented Jul 27, 2017

Uh oh!

zayass commented Jul 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ianpartridge commented Jul 27, 2017

Uh oh!

zayass left a comment

Choose a reason for hiding this comment

Uh oh!

zayass Jul 27, 2017

Choose a reason for hiding this comment

Uh oh!

ianpartridge commented Jul 27, 2017

Uh oh!

alblue commented Jul 28, 2017

Uh oh!

ianpartridge Jul 28, 2017

Choose a reason for hiding this comment

Uh oh!

ianpartridge Jul 28, 2017

Choose a reason for hiding this comment

Uh oh!

ianpartridge Jul 28, 2017

Choose a reason for hiding this comment

Uh oh!

ianpartridge commented Jul 28, 2017

Uh oh!

zayass commented Jul 28, 2017

Uh oh!

ianpartridge commented Jul 28, 2017

Uh oh!

ianpartridge commented Jul 28, 2017

Uh oh!

zayass commented Jul 31, 2017

Uh oh!

ianpartridge commented Jul 31, 2017

Uh oh!

Uh oh!

xwu Jul 27, 2017 •

edited

Loading

zayass commented Jul 27, 2017 •

edited

Loading