Restore initializing entry points for @objc convenience initializers #21815
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
jrose-apple
commented
Jan 12, 2019
|
|
||
| convenience init(swiftToNormalFactory: ()) { | ||
| // FIXME: This shouldn't be allowed, since the factory won't actually use | ||
| // the dynamic Self type. |
There was a problem hiding this comment.
Filed as SR-9645, though it's not obvious what the answer is yet.
There was a problem hiding this comment.
Does it error based on -swift-version? I believe Slava made it so that all inits formally return Self only in -swift-version 5.
There was a problem hiding this comment.
It, uh, fails the SIL verifier in -swift-version 5. Oops. I guess that means we can make it an error there?
There was a problem hiding this comment.
We could do that in a follow-up patch, maybe.
|
Build failed |
|
Build failed |
This undoes some of Joe's work in 8665342 to add a guarantee: if an @objc convenience initializer only calls other @objc initializers that eventually call a designated initializer, it won't result in an extra allocation. While Objective-C /allows/ returning a different object from an initializer than the allocation you were given, doing so doesn't play well with some very hairy implementation details of compiled nib files (or NSCoding archives with cyclic references in general). This guarantee only applies to (1) calling `self.init` (2) where the delegated-to initializer is @objc because convenience initializers must do dynamic dispatch when they delegate, and Swift only stores allocating entry points for initializers in a class's vtable. To dynamically find an initializing entry point, ObjC dispatch must be used instead. (It's worth noting that this patch does NOT check that the calling initializer is a convenience initializer when deciding whether to use ObjC dispatch for `self.init`. If we ever add peer delegation to designated initializers, which is totally a valid feature, that should use static dispatch and therefore should not go through objc_msgSend.) This change doesn't /always/ result in fewer allocations; if the delegated-to initializer ends up returning a different object after all, the original allocation was wasted. Objective-C has the same problem (one of the reasons why factory methods exist for things like NSNumber and NSArray). We do still get most of the benefits of Joe's original change. In particular, vtables only ever contain allocating initializer entry points, never the initializing ones, and never /both/ (which was a thing that could happen with 'required' before). rdar://problem/46823518
jrose-apple
force-pushed
the
collocating-convenience-initializers
branch
from
5c544c6 to
5f9297a
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
Build failed |
|
Build failed |
jrose-apple
added a commit
to jrose-apple/swift
that referenced
this pull request
Jan 14, 2019
…wiftlang#21815) This undoes some of Joe's work in 8665342 to add a guarantee: if an @objc convenience initializer only calls other @objc initializers that eventually call a designated initializer, it won't result in an extra allocation. While Objective-C /allows/ returning a different object from an initializer than the allocation you were given, doing so doesn't play well with some very hairy implementation details of compiled nib files (or NSCoding archives with cyclic references in general). This guarantee only applies to (1) calling `self.init` (2) where the delegated-to initializer is @objc because convenience initializers must do dynamic dispatch when they delegate, and Swift only stores allocating entry points for initializers in a class's vtable. To dynamically find an initializing entry point, ObjC dispatch must be used instead. (It's worth noting that this patch does NOT check that the calling initializer is a convenience initializer when deciding whether to use ObjC dispatch for `self.init`. If we ever add peer delegation to designated initializers, which is totally a valid feature, that should use static dispatch and therefore should not go through objc_msgSend.) This change doesn't /always/ result in fewer allocations; if the delegated-to initializer ends up returning a different object after all, the original allocation was wasted. Objective-C has the same problem (one of the reasons why factory methods exist for things like NSNumber and NSArray). We do still get most of the benefits of Joe's original change. In particular, vtables only ever contain allocating initializer entry points, never the initializing ones, and never /both/ (which was a thing that could happen with 'required' before). rdar://problem/46823518 (cherry picked from commit 425c190)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This undoes some of Joe's work in #19151 to add a guarantee: if an
@objcconvenience initializer only calls other@objcinitializers that eventually call a designated initializer, it won't result in an extra allocation. While Objective-C allows returning a different object from an initializer than the allocation you were given, doing so doesn't play well with some very hairy implementation details of compiled nib files (or NSCoding archives with cyclic references in general).This guarantee only applies to
self.init@objcbecause convenience initializers must do dynamic dispatch when they delegate, and Swift only stores allocating entry points for initializers in a class's vtable. To dynamically find an initializing entry point, ObjC dispatch must be used instead.
(It's worth noting that this patch does NOT check that the calling initializer is a convenience initializer when deciding whether to use ObjC dispatch for
self.init. If we ever add peer delegation to designated initializers, which is totally a valid feature, that should use static dispatch and therefore should not go through objc_msgSend.)This change doesn't always result in fewer allocations; if the delegated-to initializer ends up returning a different object after all, the original allocation was wasted. Objective-C has the same problem (one of the reasons why factory methods exist for things like NSNumber and NSArray).
We do still get most of the benefits of Joe's original change. In particular, vtables only ever contain allocating initializer entry points, never the initializing ones, and never both (which was a thing that could happen with
requiredbefore).rdar://problem/46823518