Test that newer libobjcs let the Swift runtime set up a class's layout #21942
Conversation
|
Either of you see anything else worth testing while I'm here? |
jrose-apple
force-pushed
the
layout-is-french-for-exit
branch
from
52fb1a4 to
8e1a60e
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
| @objc var first: Int = 0 | ||
| var forceUnalignment: Int8 = 0 | ||
| var middle = GrowsToInt64() | ||
| @objc var last: Int = 0 |
There was a problem hiding this comment.
Why are these (non-public) ivars @objc?
There was a problem hiding this comment.
These are the ones I'm testing the ivar offsets of. It doesn't really matter if they're public or not; Swift never exposes direct compile-time ivar access to Objective-C.
There was a problem hiding this comment.
If we ever don't emit them for non-@objc ivars, I don't want the test to change. I think it's appropriate for something accessed through the ObjC runtime. (Besides, the non-@objc ivars aren't representable in Objective-C, and it feels weird to rely on them being there.)
|
|
||
| size_t offsetOfLast = ivar_getOffset(class_getInstanceVariable(c, "last")); | ||
| NSLog(@"offset of 'last' %zd", offsetOfLast); | ||
| assert(offsetOfLast == actualSize - sizeof(intptr_t)); |
There was a problem hiding this comment.
Is it worth creating an instance and checking anything there as well, maybe calling a Swift method too?
There was a problem hiding this comment.
I couldn't think of anything that wouldn't go through a Swift entry point except class_createInstance, and I think that's superfluous with checking the instance size.
There was a problem hiding this comment.
Yeah, the case I thought we could test is calling Swift code after Objective-C had already initialized the class metadata
There was a problem hiding this comment.
Well, we call static methods. Think an instance method is interesting enough to also be worth doing?
|
|
||
| size_t offsetOfFirst = ivar_getOffset(class_getInstanceVariable(c, "first")); | ||
| NSLog(@"offset of 'first' %zd", offsetOfFirst); | ||
| assert(offsetOfFirst == sizeof(Class) + (isPureSwift ? sizeof(void*) : 0)); |
There was a problem hiding this comment.
Does this still hold on 32-bit? (I guess we'll find out)
There was a problem hiding this comment.
Yeah, this is "non-pointer isa" or "isa + refcount". I checked on a 32-bit simulator.
| } | ||
| } | ||
|
|
||
| public class DynamicClass: NSObject { |
There was a problem hiding this comment.
Can you also test a subclass of an Objective-C class that is not NSObject (and has ivars)? We've had bugs with ivar layout in this case in the past.
There was a problem hiding this comment.
Good idea, will add.
There was a problem hiding this comment.
Actually, if I do that I can give all three classes the same layout, which would be nice for maintenance purposes.
|
This looks good. Thanks for doing this! |
| #elseif BIG | ||
| var value: Int64 | ||
| #else | ||
| #error("Must define OLD or NEW") |
There was a problem hiding this comment.
Whoops, thanks!
jrose-apple
force-pushed
the
layout-is-french-for-exit
branch
from
8e1a60e to
60e07aa
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
Looks like I got the 32-bit stuff wrong after my second version. Fortunately my 32-bit simulator is working now…! |
Specifically, when the class is referenced from Objective-C first, before being referenced from Swift in any way. This is important for resilience, since up until now Objective-C isn't used to the size of a class changing based on anything other than a superclass's size changing. This is basically the test that would have gone with 9024768 had there been a build of libobjc to test it with. rdar://problem/45718008
jrose-apple
force-pushed
the
layout-is-french-for-exit
branch
from
60e07aa to
cd00a8c
Compare
|
@swift-ci Please test macOS |
|
@swift-ci Please smoke test Linux |
|
Build failed |
swiftlang#21942) Specifically, when the class is referenced from Objective-C first, before being referenced from Swift in any way. This is important for resilience, since up until now Objective-C isn't used to the size of a class changing based on anything other than a superclass's size changing. This is basically the test that would have gone with 9024768 had there been a build of libobjc to test it with. rdar://problem/45718008 (cherry picked from commit 7629253)
Specifically, when the class is referenced from Objective-C first, before being referenced from Swift in any way. This is important for resilience, since up until now Objective-C isn't used to the size of a class changing based on anything other than a superclass's size changing.
This is basically the test that would have gone with #19944 had there been a build of libobjc to test it with.
rdar://problem/45718008