-
Notifications
You must be signed in to change notification settings - Fork 339
[lldb] Resolve Swift-implemented Objective-C classes using Swift runtime #10548
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
base: swift/release/6.2
Are you sure you want to change the base?
Conversation
lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp
Outdated
Show resolved
Hide resolved
@swift-ci test |
@swift-ci test |
This is now ready to review. |
@swift-ci test |
@@ -2172,10 +2198,20 @@ bool SwiftLanguageRuntime::GetDynamicTypeAndAddress_Class( | |||
return false; | |||
} | |||
} | |||
|
|||
LLDB_LOG(log, "dynamic type of instance_ptr {0:x} is {1}", instance_ptr, | |||
LLDB_LOG(GetLog(LLDBLog::Types), |
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.
These variables are not set at the point that this log is printed right?
dynamic_type.GetTypeSystem().dyn_cast_or_null<TypeSystemSwift>(); | ||
if (ts && | ||
ts->IsImportedType(dynamic_type.GetOpaqueQualType(), nullptr)) | ||
type_name = static_type.GetDisplayTypeName(); |
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.
Do we always want to use the static type if the dynamic type is an Objective-C type? I guess in the REPL this makes sense
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.
Right, this is a very blunt tool to hide some of the Foundation implementation detail types that users are not meant to see / know / care about.
It doesn't not work for Swift-implemented types like _SwiftDeferredNSDictionary
, which is going to be more prevalent going forward.
auto tss = class_type.GetTypeSystem().dyn_cast_or_null<TypeSystemSwift>(); | ||
if (!tss) { | ||
is_clang_type = true; | ||
if (auto module_sp = in_value.GetModule()) { |
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 understand why this is needed. If the type does not have a Swift type system (even if it's a swiftified objc type), then something has gone wrong, and we shouldn't be asking the Swift Language Runtime, no?
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.
Good question! Let me try replacing this with an assert(false) to answer how we get here.
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 answer is
bool ValueObjectDynamicValue::UpdateValue() {
...
#ifdef LLDB_ENABLE_SWIFT
// An Objective-C object inside a Swift frame.
if (known_type == eLanguageTypeObjC)
if ((exe_ctx.GetFramePtr() && exe_ctx.GetFramePtr()->GetLanguage().name ==
llvm::dwarf::DW_LNAME_Swift) ||
(exe_ctx.GetTargetPtr() && exe_ctx.GetTargetPtr()->IsSwiftREPL())) {
runtime = process->GetLanguageRuntime(lldb::eLanguageTypeSwift);
if (runtime)
found_dynamic_type = runtime->GetDynamicTypeAndAddress(
*m_parent, m_use_dynamic, class_type_or_name, dynamic_address,
value_type, local_buffer);
}
#endif // LLDB_ENABLE_SWIFT
let me see if I can simplify this.
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 was able to eliminate the entire resolve_objc
lambda. The interesting part is that it was also trying to solve the private implementation detail problem, however, this was no longer working.
if the Objective-C runtime fails. If an Objective-C class is lazy, the Objective-C runtie may not have materialized class metadata for it. However, if the class is actually implemented in Swift, we can still resolve it using the Swift runtime. We should probably also add the same logic to the Objective-C runtime, but I don't want risk adding an inifinite recursion at this point in the release.
@swift-ci test |
if the Objective-C runtime fails. If an Objective-C class is lazy, the
Objective-C runtie may not have materialized class metadata for
it. However, if the class is actually implemented in Swift, we can
still resolve it using the Swift runtime. We should probably also add
the same logic to the Objective-C runtime, but I don't want risk
adding an inifinite recursion at this point in the release.