Eliminate Builtin.UnknownObject as an AST type #27378
Conversation
|
apple/swift-lldb#2017 |
|
@swift-ci Please test source compatibility |
|
Build failed |
|
Build failed |
|
apple/swift-lldb#2017 |
|
Build failed |
|
|
||
| bb1(%8 : $@convention(objc_method) (Builtin.UnknownObject) -> ()): | ||
| bb1(%8 : $@convention(objc_method) (@opened("01234567-89ab-cdef-0123-000000000000") AnyObject) -> ()): |
There was a problem hiding this comment.
These updates match how dynamic_method_br is emitted these days.
| @@ -76,7 +76,7 @@ enum MultiPayloadGeneric<T> { | |||
| enum MultiPayloadNontrivialSpareBits { | |||
| case payload1(Builtin.NativeObject) | |||
| case payload2(Builtin.Int64) | |||
| case payload3(Builtin.Int64, Builtin.UnknownObject) | |||
| case payload3(Builtin.Int64, Builtin.NativeObject) | |||
There was a problem hiding this comment.
These tests relied on having spare bits in UnknownObject, which was a questionable assumption. (Good thing we weren't actually using it!)
| // CHECK: br label %9 | ||
|
|
||
| // CHECK: ; <label>:9: | ||
| // CHECK: ret void |
There was a problem hiding this comment.
These tests also relied on having spare bits in UnknownObject. They don't seem to offer any additional coverage now.
| %6 = open_existential_ref %4 : $AnyObject to $@opened("01234567-89ab-cdef-0123-000000000000") AnyObject | ||
| %7 = unchecked_ref_cast %6 : $@opened("01234567-89ab-cdef-0123-000000000000") AnyObject to $Builtin.UnknownObject | ||
| dynamic_method_br %7 : $Builtin.UnknownObject, #X.f!1.foreign, bb1, bb2 | ||
| dynamic_method_br %4 : $AnyObject, #X.f!1.foreign, bb1, bb2 |
There was a problem hiding this comment.
This one doesn't quite match what we emit today but I figured the test relied on the two branches having the same type.
| @@ -142,17 +142,16 @@ bb0: | |||
| // Now check that a native object address may: | |||
| // | |||
| // 1. May alias raw pointer addresses. | |||
| // 2. Not alias an unknown object. Objective-C objects are allocated differently | |||
| // than Swift objects and may not alias. | |||
| // 2. May alias an unknown object. | |||
There was a problem hiding this comment.
OOPS GOOD THING WE WEREN'T USING THIS
This removes it from the AST and largely replaces it with AnyObject at the SIL and IRGen layers. Some notes: - Reflection still uses the notion of "unknown object" to mean an object with unknown refcounting. There's no real reason to make this different from AnyObject (an existential containing a single object with unknown refcounting), but this way nothing changes for clients of Reflection, and it's consistent with how native objects are represented. - The value witness table and reflection descriptor for AnyObject use the mangling "BO" instead of "yXl". - The demangler and remangler continue to support "BO" because it's still in use as a type encoding, even if it's not an AST-level Type anymore. - Type-based alias analysis for Builtin.UnknownObject was incorrect, so it's a good thing we weren't using it. - Same with enum layout. (This one assumed UnknownObject never referred to an Objective-C tagged pointer. That certainly wasn't how we were using it!)
jrose-apple
force-pushed
the
know-it-all
branch
from
696bb78 to
3a655a5
Compare
|
That honestly went better than expected, this should be ready now. I hope it doesn't muck up Instruments or anything though. (Don't worry, watchers, it'll be tested additionally before it ships in an Xcode.) apple/swift-lldb#2017 |
|
Build failed |
|
Build failed |
jrose-apple
changed the title
|
Credit to @varungandhi-apple for getting me to try this again by running into a problem with Builtin.UnknownObject (even if he's already unblocked himself). |
|
@atrick or @aschwaighofer, can you check that the test changes I called out above are reasonable? |
|
The test case cleanup all looks good. |
New version of #12325 that doesn't attempt to change the runtime and reflection bits.
Still needs better comments, more docs, etc.