-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle resilient stored properties in objcImpl #73128
Conversation
10009b0
to
b0fe8d4
Compare
Because this test is disabled, a previous commit failed to add an experimental feature flag that’s required to run it.
Rather than having individual methods return without doing anything, arrange for ClassMetadataVisitor to never call them in the first place. This makes it so ClassMetadataScanner also knows which fields are omitted and it can compute offsets correctly. The old logic is still present for field offsets to make this change NFC.
b0fe8d4
to
6a44e8f
Compare
d1e9c47
to
9f4f051
Compare
@swift-ci please test |
1 similar comment
@swift-ci please test |
175c244
to
fb60673
Compare
@swift-ci please test |
fb60673
to
64974f4
Compare
@swift-ci please test |
// Collect the stored properties of the type. | ||
unsigned numFields = getNumFields(target); | ||
|
||
// Fill out an array with the field type metadata records. | ||
Address fields = IGF.createAlloca( | ||
Address fields = createAlloca( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: it looks like some code in this file hasn't been reformatted after changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I initially wasn't sure if this change would stick. I should go over this again.
// Update and FixedOrUpdate require support in both the Swift and ObjC | ||
// runtimes. | ||
auto requiredAvailability=ctx.getUpdatePureObjCClassMetadataAvailability(); | ||
// FIXME: Take the class's availability into account |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, does class metadata layout happen lazily when the first instance is created or something? I'm wondering how gating this on the availability of the class itself would be safe in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first time the ObjC runtime tries to access any of the metadata associated with a class—basically, use it as anything but a completely opaque pointer—it performs an operation called "realization". The metadata update function that Swift generates will be called during realization, and it will call the new swift_updatePureObjCClassMetadata()
function at that time. So realization might happen when you try to look up the +alloc
method to allocate an instance of the class, or it could happen even earlier, depending on how the code uses the class.
@@ -1,34 +1,34 @@ | |||
// Test doesn't pass on all platforms (rdar://101420862) | |||
// REQUIRES: OS=macosx | |||
|
|||
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-experimental-feature CImplementation -I %S/Inputs/abi -F %clang-importer-sdk-path/frameworks %s -import-objc-header %S/Inputs/objc_implementation.h -emit-ir > %t.ir | |||
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-experimental-feature CImplementation -I %S/Inputs/abi -F %clang-importer-sdk-path/frameworks %s -import-objc-header %S/Inputs/objc_implementation.h -emit-ir -target %target-future-triple > %t.ir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it would make sense here, but we now have lit substitutions like %target-swift-abi-6.0-triple
if you wanted to specify a specific, supported deployment target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we technically know yet when this will land, so future availability seems most appropriate for the moment.
When an @objc @implementation class requires the use of `ClassMetadataStrategy::Update` because some of its stored properties do not have fixed sizes, we adjust the direct field offsets during class realization by emitting a custom metadata update function which calls a new entry point in the Swift runtime. That entry point adjusts field offsets like `swift_updateClassMetadata2()`, but it only assumes that the class has Objective-C metadata, not Swift metadata. This commit introduces an alternative mechanism which does the same thing without using any Swift-only metadata. It’s a rough implementation with important limitations: • We’re currently using the field offset vector, which means that field offsets are being emitted into @objc @implementation classes; these will be removed. • The new Swift runtime entry point duplicates a lot of `swift_updateClassMetadata2()`’s implementation; it will be refactored into something much smaller and more compact. • Availability bounds for this feature have not yet been implemented. Future commits in this PR will correct these issues.
We really don’t need ‘em; we can just adjust the direct field offsets. The runtime entry point currently uses a weird little hack that we will refactor away shortly.
Merge the three-stage operation originally designed for field vectors into a single unified loop that acts directly on the ivar offsets instead of using a faux field offset vector.
We’re not committing to @objc @implementation back-deploying to pre-stable Apple platforms.
…when the deployment target is not high enough to support them.
Resilence support will require changes to the Objective-C runtime to expand support for metadata initialization functions. Add a separate experimental feature flag to help with staging that support in, and modify diagnostics to not suggest increasing the minimum deployment target for now.
64974f4
to
6d3e1ad
Compare
Made some formatting improvements at @tshortli's request. No functional changes. |
@swift-ci please smoke test |
Windows build timed out. |
@swift-ci please smoke test Windows platform |
This PR temporarily bans, and lays the groundwork for eventually supporting, resilient stored properties in
@objc @implementation
classes.When a stored property contains a resilient value type, its size cannot be determined at compile time. That means the size and exact layout of the instance variables is not known, either. The layout must be computed, and the instance variables and class size data updated, when the class is realized (i.e. at runtime, but before it is used).
For Swift classes (including ordinary
@objc
classes), the mechanism used to do this works like so:swift_updateClassMetadata2()
function to update the instance variable layout and tell the ObjC runtime to finish realizing the class.This happens before the Objective-C runtime does its own ivar sliding. In effect, the ivars are slid twice—once to account for changes in the sizes of other ivars ahead of them in the class layout, then again to account for changes in the size of the superclass.
Unfortunately, every single step in this chain is designed for full-fledged Swift classes with Swift metadata; all of them break horribly if you try to use them with a "pure ObjC" class like the ones created by
@objc @implementation
:swift_updateClassMetadata2()
function assumes it has been passed a pointer to Swift class metadata and both reads and writes fields that are not present in pure ObjC classes.This PR lays the groundwork for an alternate mechanism to update the layout of a pure ObjC class during class realization:
swift_updatePureObjCClassMetadata()
, which uses only the Objective-C metadata. This sidesteps problem 4.However, problem 1 will require changes to the Objective-C runtime, so the new code generation is gated behind an experimental feature flag,
ObjCImplementationWithResilientStorage
. Without that flag, Swift will unconditionally reject properties that would require this new mechanism; with it, Swift will enforce a (for now) 99.99 minimum deployment target for it.Fixes rdar://126948464.