-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb] Update dwim-print to show expanded objc instances #117500
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
Merged
kastiglione
merged 5 commits into
llvm:main
from
kastiglione:lldb-Update-dwim-print-to-show-expanded-objc-instances
Mar 15, 2025
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3199d3e
[lldb] Update dwim-print to show expanded objc instances
kastiglione 1890aaa
python formatting
kastiglione 40aadad
Apply skipUnlessDarwin decorator
kastiglione 668e91d
Test expansion of nested objc objects
kastiglione 0e8ddf0
Test for no expansion when summary formatter is present
kastiglione File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
OBJC_SOURCES := main.m | ||
LD_EXTRAS := -framework Foundation | ||
include Makefile.rules |
27 changes: 27 additions & 0 deletions
27
lldb/test/API/commands/dwim-print/objc/TestDWIMPrintObjC.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
""" | ||
Test dwim-print with objc instances. | ||
""" | ||
|
||
import lldb | ||
from lldbsuite.test.lldbtest import * | ||
from lldbsuite.test.decorators import * | ||
import lldbsuite.test.lldbutil as lldbutil | ||
|
||
|
||
class TestCase(TestBase): | ||
@skipUnlessDarwin | ||
def test(self): | ||
self.build() | ||
lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m")) | ||
self.expect("dwim-print parent", substrs=["_child = 0x"]) | ||
self.expect( | ||
"dwim-print parent.child", patterns=[r'_name = 0x[0-9a-f]+ @"Seven"'] | ||
) | ||
|
||
@skipUnlessDarwin | ||
def test_with_summary(self): | ||
self.build() | ||
lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m")) | ||
self.runCmd("type summary add -s 'Parent of ${var._child._name}' 'Parent *'") | ||
self.expect("dwim-print parent", matching=False, substrs=["_child = 0x"]) | ||
self.expect("dwim-print parent", substrs=['Parent of @"Seven"']) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
#import <Foundation/Foundation.h> | ||
|
||
@interface Child : NSObject | ||
@property(nonatomic, copy) NSString *name; | ||
@end | ||
|
||
@implementation Child | ||
@end | ||
|
||
@interface Parent : NSObject | ||
@property(nonatomic, strong) Child *child; | ||
@end | ||
|
||
@implementation Parent | ||
@end | ||
|
||
int main(int argc, char **argv) { | ||
Child *child = [Child new]; | ||
child.name = @"Seven"; | ||
Parent *parent = [Parent new]; | ||
parent.child = child; | ||
puts("break here"); | ||
return 0; | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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 this something we would want for
frame var
/expr
too? In which case we could just check for ObjC inValueObjectPrinter::ShouldPrintChildren
directly instead of introducing this new APIThere 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.
it definitely should be considered, but I didn't want to impede this change with those possibly more debatable changes. I have more thoughts on this, I'll write them up later today or this week.
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.
Here's my thinking: most of the time, I want top level pointers to be expanded (as c++ references are). But each of the C based language has its considerations.
For ObjC, I think all top level objects should be expanded, and I can't think of any counter arguments.
For C++, I also want top level pointers to be expanded – however raw pointers are less common. In C++ I would like to see top level smart pointers be expanded.
For C, I am less sure what, if anything, to do about top level pointers. First, I don't write a lot of C and have less experience to form opinions. Second, in C a pointer might be just as likely to be a buffer/array, and expanding a buffer as a single pointer would be misleading.
Coming back to your question: I would like to see C++ raw pointers expanded, and hopefully smart pointers too. I would leave C pointers alone. I guess I need to raise this on the forums, and find some buy in. As part of that, we can determine should pointer expansion be specific to
dwim-print
, or should some/all of it apply toframe var
/expr
too.Uh oh!
There was an error while loading. Please reload this page.
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'm not actually sure what I'd prefer for C++. Usually when I print a pointer I only what to see the pointer value, not it's dereferenced form. But maybe that's because I've gotten used to the LLDB CLI and the way
frame var
works.This thread here might be relevant: #117755 (review)
Limiting this change to ObjC for now seems fine to me. But yea, asking a wider audience might be a good idea.
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.
Should/Could this perhaps be an (NSObject *) data formatter, doing this work, so it's applied consistently?
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.
@adrian-prantl I tried this (with a Python implementation) and that didn't work for two reasons:
NSObject *
did not result in that formatter being used withSubclass *
v -P1 <variable>