Remove unsafeAddress(of:) #3644
Conversation
|
@gribozavr Do you mind reviewing this patch? |
| @@ -296,7 +296,7 @@ internal func reflect(instanceAddress: UInt, kind: InstanceKind) { | |||
| /// This reflects the stored properties of the immediate class. | |||
| /// The superclass is not (yet?) visited. | |||
| public func reflect(object: AnyObject) { | |||
| let address = unsafeAddress(of: object) | |||
| var address = Unmanaged.passUnretained(object).toOpaque() | |||
There was a problem hiding this comment.
This two pieces of code in SwiftReflectionTest.swift look buggy to me. They need at least a fixLifetime, could you add it? (defer { _fixLifetime(object) } at the beginning of the method.)
|
Just small comments, LGTM otherwise! Thank you! |
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
@gribozavr Is it what you meant? Specially, is it correct to put @_version just before the non _ObjC version?
There was a problem hiding this comment.
Yes, I think that's the right way to do it.
|
@gribozavr I incorporated all of your comments and force-pushed the new version. |
|
@swiftix LGTM, thank you! |
|
@gribozavr BTW, do we need to provide a deprecated attribute with a message like: "Unmanaged.passUnretained(x).toOpaque()" instead? Or do we just silently remove the APIs? |
|
@swiftix Yes, it would be great if we could provide migration attributes for both |
|
@gribozavr OK, I'll add those to the methods and use |
gribozavr
added
the
swift evolution pending discussion
|
@swift-ci Please test |
|
This is formally approved by means of a swift-evolution process now. |
gribozavr
changed the title
|
@parkera This PR is removing a |
|
@parkera Forgot to provide the link to the compilation errors |
|
@swiftix Put up a PR on swift-corelibs-foundation and I'll merge it. |
|
@swift-ci Please smoke test Linux. |
|
@swift-ci Please test |
rdar://problem/18589289
|
@swift-ci Please smoke test Linux. |
|
Test failures seem to be unrelated. |
|
@gribozavr Can you please review for CCC? There are virtually no changes since last review. I just fixed merge conflicts and force-pushed again. |
|
LGTM. |
AnthonyLatsis
removed
the
swift evolution pending discussion
What's in this pull request?
The is a patch for the upcoming Swift evolution proposal.
rdar://problem/18589289
Resolves #44566