-
Notifications
You must be signed in to change notification settings - Fork 353
[LLDB] Add ObjC object typeref support to SwiftRuntimeTypeVisitor #11929
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.3
Are you sure you want to change the base?
Conversation
0e52023 to
646d2f3
Compare
|
@swift-ci test |
646d2f3 to
29e2381
Compare
|
Updated the test, the previous one didn't actually exercise the new code m-( |
|
@swift-ci test |
29e2381 to
072a092
Compare
072a092 to
a58c094
Compare
|
@swift-ci test |
|
|
||
| if (!ts) | ||
| return llvm::createStringError("no clang type system"); | ||
| clang::EnumDecl *enum_decl = ts->GetAsEnumDecl(clang_type); |
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.
Is it possible to refactor this in a way where we don't directly manipulate the clang decls in swift language runtime?
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.
Ideally we'd be able to move a lot of this function to the clang type system, since this is a clang type after all.
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.
Maybe, I think it's reasonable for the parts that deal with Clang interoperability to rely on Clang-specific details.
| if (!n_or_err) | ||
| return n_or_err.takeError(); | ||
| unsigned n = *n_or_err; | ||
| bool is_signed, is_enum = clang_type.IsEnumerationType(is_signed); |
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.
Is the usage of is_signed here correct? You're both declaring it and passing it to IsEnumerationType.
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.
this is equivalent to
bool is_signed;
bool is_enum = clang_type.IsEnumerationType(is_signed);
| return bit_size.takeError(); | ||
| ChildInfo child; | ||
| child.byte_size = *bit_size / 8; | ||
| child.byte_offset = 0; |
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.
Is byte_offset = 0 really correct here? If this is a struct with an objc field won't this cause problems?
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.
This is the is_enum case.
lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeDynamicTypeResolution.cpp
Outdated
Show resolved
Hide resolved
89ea55b to
d1706f5
Compare
This patch moves the disjunct implementations for TypeSystemTypeRef::GetNumFields and GetIndexOfChildMemberWithName into the TypeRef visitor, so they behave consistently. rdar://154450536
d1706f5 to
4e2b9ed
Compare
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
This is done by defering to the Objective-C runtime. This functionality is implemented in the visitor because ObjC object typerefs are fully valid reference types in the Swift reflection descriptors already and only LLDB wants to look inside them.
rdar://154450536