Skip to content

[lldb] Support plain ObjC names in LLDBTypeInfoProvider #9320

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

Conversation

kastiglione
Copy link

@kastiglione kastiglione commented Sep 24, 2024

Extend LLDBTypeInfoProvider to support ObjC class names (such as "NSObject", "NSView") in addition to mangled Swift names.

This is to support ObjCClassTypeRefs, which contain the class name, but not the mangled name. Note that ObjCClassTypeRefs are typerefs for classes in the ObjC (__C) module.

Depends on swiftlang/swift#76678

@kastiglione kastiglione marked this pull request as draft September 24, 2024 17:55
@kastiglione kastiglione marked this pull request as ready for review September 26, 2024 17:21
@kastiglione
Copy link
Author

swiftlang/swift#76678

@swift-ci test

self, "// break here", lldb.SBFileSpec("main.swift")
)
self.runCmd("settings set symbols.swift-enable-ast-context false")
self.expect("v", substrs=["num = 15"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Usually I prefer frame variable c over v since v is technically an alias, so there is a chance that it will be re-aliased later on to something else.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be nice to have a policy on this. The v alias could change, but it would have to be a breaking change for a test like this to be a problem. A breaking change seems unlikely.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other instances, I'll make a follow up change to catch them all.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kastiglione
Copy link
Author

swiftlang/swift#76678

@swift-ci test

@kastiglione kastiglione merged commit 4a1abae into stable/20230725 Sep 27, 2024
3 checks passed
@kastiglione kastiglione deleted the dl/lldb-Support-plain-ObjC-names-in-LLDBTypeInfoProvider branch September 27, 2024 18:11
kastiglione added a commit to swiftlang/swift that referenced this pull request Sep 27, 2024
…ffset (#76678)

This change is part of a fix for a regression that wasn't noticed because lldb has a 
fallback to using ASTContexts when type metadata resolution fails.

For any Swift class that directly or indirectly inherits from an ObjC class (such as 
`NSObject`, `NSView`, etc), the function `computeUnalignedFieldStartOffset` will fail 
to compute the start offset. To compute the start offset, it traverses the parent 
classes and uses their sizes. Once the traversal reaches an ObjC class, the 
RemoteInspection does not know the size. The fix here is to get the size from the 
`TypeInfo` returned by the external `TypeInfoProvider` (which in LLDB is 
`LLDBTypeInforProvider`).

Depended on by swiftlang/llvm-project#9320
@adrian-prantl
Copy link

It would be nice to squash follow-up fixes if they don't make sense on their own. Makes cherry-picking etc easier.

@adrian-prantl
Copy link

Oh wait. This is even more confusing. You did! But Github only shows the original commits.

adrian-prantl pushed a commit to adrian-prantl/llvm-project that referenced this pull request Sep 27, 2024
Extend `LLDBTypeInfoProvider` to support ObjC class names (such as "NSObject",
"NSView") in addition to mangled Swift names.

This is to support `ObjCClassTypeRefs`, which contain the class name, but not the
mangled name. Note that `ObjCClassTypeRefs` are typerefs for classes in the ObjC
(`__C`) module.

Depends on swiftlang/swift#76678

(cherry picked from commit 4a1abae)
adrian-prantl added a commit that referenced this pull request Sep 27, 2024
[lldb] Support plain ObjC names in LLDBTypeInfoProvider (#9320)
kastiglione added a commit that referenced this pull request Sep 30, 2024
Extend `LLDBTypeInfoProvider` to support ObjC class names (such as "NSObject",
"NSView") in addition to mangled Swift names.

This is to support `ObjCClassTypeRefs`, which contain the class name, but not the
mangled name. Note that `ObjCClassTypeRefs` are typerefs for classes in the ObjC
(`__C`) module.

Depends on swiftlang/swift#76678

(cherry picked from commit 4a1abae)
kastiglione added a commit that referenced this pull request Sep 30, 2024
Extend `LLDBTypeInfoProvider` to support ObjC class names (such as "NSObject", 
"NSView") in addition to mangled Swift names.

This is to support `ObjCClassTypeRefs`, which contain the class name, but not the 
mangled name. Note that `ObjCClassTypeRefs` are typerefs for classes in the ObjC 
(`__C`) module.

Depends on swiftlang/swift#76678

(cherry picked from commit 4a1abae)
kastiglione added a commit that referenced this pull request Oct 4, 2024
This change is for two reasons:

> prefer `frame variable` over `v` since `v` is technically an alias, so there is a 
chance that it will be re-aliased later on to something else.
See #9320 (comment)

Additionally, using `frame var` makes it easier to grep for than using `v`.
kastiglione added a commit that referenced this pull request Oct 29, 2024
This change is for two reasons:

> prefer `frame variable` over `v` since `v` is technically an alias, so there is a 
chance that it will be re-aliased later on to something else.
See #9320 (comment)

Additionally, using `frame var` makes it easier to grep for than using `v`.

(cherry-picked from commit ab5b104)
kastiglione added a commit that referenced this pull request Oct 29, 2024
This change is for two reasons:

> prefer `frame variable` over `v` since `v` is technically an alias, so there is a 
chance that it will be re-aliased later on to something else.
See #9320 (comment)

Additionally, using `frame var` makes it easier to grep for than using `v`.

(cherry-picked from commit ab5b104)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants