[5.0] Restore initializing entry points for @objc convenience initializers #21849
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 |
|
Build failed |
|
Oops, right. "ossa" post-dates the Swift 5 branch. |
|
Holding also for bug fix discovered in the original PR, rdar://problem/47266149. |
|
Build failed |
…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)
jrose-apple
force-pushed
the
5.0-collocating-convenience-initializers
branch
from
5fdf7e4 to
be55564
Compare
The optimizer was smart enough to stack-promote the test objects I was using, which foiled my interposition on swift_allocObject. Change the tests to "escape" each allocation by returning it. rdar://problem/47266149 (cherry picked from commit 80ef1f5)
|
Build failed |
|
Build failed |
|
Source compat checks passed before the test fix, save for the NonEmpty project that's currently UPASSing. |
AnnaZaks
approved these changes
Jan 15, 2019
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.
Explanation: Early in the Swift 5 release, @jckarter simplified Swift's implementation of convenience initializers to always go through the "allocating" entry point—in practice, delaying the responsibility of allocating
selfuntil necessary, which in practice means "until calling a designated initializer, or an initializer implemented in Objective-C". This had a number of benefits for the rest of the compiler, the primary one being a significant simplification in how vtables are built (which also manifests as a minor code size win). However, it also broke the decoding of object graphs with circular references through NSCoding, including Interface Builder files, because the unarchiver object can't know that its initial allocation (through Objective-C's+allocWithZone:) is going to be replaced. (That is, it breaks it when the implementation ofinit(coder:)in Swift is a convenience initializer.)To restore this capability, allow initializer delegating (
self.init) to go through initializing (non-allocating) entry points when the delegated-to initializer is exposed to Objective-C. This doesn't change any safety guarantees or normally-observed behavior, and still allows the correct dynamic dispatch of the delegated-to initializer, but it does mean that as long as all initializers are@objc, the initial allocation won't be thrown away.This still isn't 100% the same as the behavior in Swift 4.2 and earlier, so it'll be worth release-noting.
Scope: Affects convenience initializers that call
@objcinitializers, as well as the definitive initialization checking for@objcconvenience initializers.Issue: rdar://problem/46823518
Risk: Medium. Most of the patch is undoing changes that Joe made, but he made them a while ago, and so we've been living on the new way.
Testing: Added compiler regression tests.
Reviewed by: @jckarter