[Swift 3.0] Eliminate double-wrapping of NSErrors in _SwiftNativeNSError #3976
Conversation
|
@swift-ci please test |
|
Does the runtime or optimizer ever do |
|
Ah, you're right that I missed a case in the runtime! The ExistentialTypeRepresentation::Error case of _dynamicCastToExistential goes straight to swift_allocError. |
…asses. In addition to using the 'Class' existential representation for NSError, use it for subclasses of NSError, which can also be toll-free bridged. Narrowly addresses rdar://problem/27658748. (cherry picked from commit 3f960e3)
…xistentials. Imported Cocoa error types are represented by structs wrapping an NSError. The conversion from these structs to Error would end up boxing the structs in _SwiftNativeNSError, losing identity and leading to a wrapping loop. Instead, extract the embedded NSError if there is one. In the Swift runtime, do this as part of the dynamic cast to NSError, using a (new, defaulted) requirement in the Error type so we can avoid an extra runtime lookup of the protocol. In SILGEn, do this by looking for the _BridgedStoredNSError protocol conformance when erasing to an Error type. Fixes SR-1562 / rdar://problem/26370984. (cherry picked from commit d219531)
When emitting an existential erasure to Error from an archetype, use the _getEmbeddedNSError() witness. If it produces an NSError, erase that; otherwise, go through the normal erasure path. Of course, make NSError and CFError implement _getEmbeddedNSError() so this kicks in for the obvious cases as well as the more obscure ones. Fixes the rest of SR-1562 / rdar://problem/26370984. (cherry picked from commit 75e85dc)
…SError. This clarifies the 'Indirect' case. Thanks, Dmitri! (cherry picked from commit 4b8b7bb)
…r existential. SILGen already attempts to extract an embedded NSError when type-erasing to an Error existential; make the runtime do the same thing dynamically. Huge thanks to Joe Groff who noticed that I missed this path. (cherry picked from commit e83fb64)
DougGregor
force-pushed
the
swift-3-nserror-wrapping
branch
from
0b177df to
171a18c
Compare
|
@swift-ci please test |
|
I'm a bit concerned about the cost of doing runtime lookup for a conformance. Building an |
|
I see, you do the lookup statically in SILGen. That won't work for a generic |
|
@jckarter , that's why there are both paths... In SILGen, we statically check for |
|
So we never dynamically look for a |
|
I see, thanks for explaining. SGTM. |
What's in this pull request?
This PR eliminates the double-wrapping of NSErrors in _SwiftNativeNSError, e.g., when an Error type that is an NSError (or subclasses NSError) or is a synthesized struct that wraps an NSError (for imported Cocoa errors) gets placed into an Error existential. Previously, these would end up getting boxed up against in a _SwiftNativeNSError, losing NSError identity and potentially causing a wrapping loop. Make sure that neither SILGen nor the runtime perform this double-wrapping.
Resolved bug number: (SR-1562)
This also resolves rdar://problem/27658748 and rdar://problem/26370984
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.