[lldb] Update dwim-print to show expanded objc instances #117500
Conversation
When printing an ObjC object, which is a pointer, lldb has handled it like it treats any other pointer, printing only pointer address and class name. The object is not expanded, and its children are not shown. This change updates the dwim-print to print expanded objc pointers - it is assumed that's what the user meant. Note that this is currently possible using the `--ptr-depth`/`-P` flag, but the default is 0. With this change, when dwim-print prints objc objects, the default is effectively changed to 1.
|
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesWhen printing an ObjC object, which is a pointer, lldb has handled it like it treats any other pointer, printing only pointer address and class name. The object is not expanded, and its children are not shown. This change updates the dwim-print to print expanded objc pointers - it is assumed that's what the user meant. Note that this is currently possible using the Full diff: https://github.com/llvm/llvm-project/pull/117500.diff 7 Files Affected:
diff --git a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
index c7f8cccc116c48..7e213f29b3bc0a 100644
--- a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
+++ b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h
@@ -125,6 +125,8 @@ class DumpValueObjectOptions {
DumpValueObjectOptions &SetRevealEmptyAggregates(bool reveal = true);
+ DumpValueObjectOptions &SetExpandPointerTypeFlags(unsigned flags);
+
DumpValueObjectOptions &SetElementCount(uint32_t element_count = 0);
DumpValueObjectOptions &
@@ -142,6 +144,7 @@ class DumpValueObjectOptions {
DeclPrintingHelper m_decl_printing_helper;
ChildPrintingDecider m_child_printing_decider;
PointerAsArraySettings m_pointer_as_array;
+ unsigned m_expand_ptr_type_flags = 0;
bool m_use_synthetic : 1;
bool m_scope_already_checked : 1;
bool m_flat_output : 1;
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 62c4e74d853ad1..8bfef15036cc7e 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -87,7 +87,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
m_expr_options.m_verbosity, m_format_options.GetFormat());
- dump_options.SetHideRootName(suppress_result);
+ dump_options.SetHideRootName(suppress_result)
+ .SetExpandPointerTypeFlags(lldb::eTypeIsObjC);
bool is_po = m_varobj_options.use_objc;
diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
index 18d590d47d9a0c..a4db0d3cb240f1 100644
--- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
+++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp
@@ -201,6 +201,12 @@ DumpValueObjectOptions::SetRevealEmptyAggregates(bool reveal) {
return *this;
}
+DumpValueObjectOptions &
+DumpValueObjectOptions::SetExpandPointerTypeFlags(unsigned flags) {
+ m_expand_ptr_type_flags = flags;
+ return *this;
+}
+
DumpValueObjectOptions &
DumpValueObjectOptions::SetElementCount(uint32_t element_count) {
m_pointer_as_array = PointerAsArraySettings(element_count);
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index face38253efab8..83db9292c5e76e 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -554,12 +554,14 @@ bool ValueObjectPrinter::ShouldPrintChildren(
return false;
const bool is_root_level = m_curr_depth == 0;
-
- if (is_ref && is_root_level && print_children) {
- // If this is the root object (depth is zero) that we are showing and
- // it is a reference, and no pointer depth has been supplied print out
- // what it references. Don't do this at deeper depths otherwise we can
- // end up with infinite recursion...
+ const bool is_expanded_ptr =
+ is_ptr && m_type_flags.Test(m_options.m_expand_ptr_type_flags);
+
+ if ((is_ref || is_expanded_ptr) && is_root_level && print_children) {
+ // If this is the root object (depth is zero) that we are showing and it
+ // is either a reference or a preferred type of pointer, then print it.
+ // Don't do this at deeper depths otherwise we can end up with infinite
+ // recursion...
return true;
}
diff --git a/lldb/test/API/commands/dwim-print/objc/Makefile b/lldb/test/API/commands/dwim-print/objc/Makefile
new file mode 100644
index 00000000000000..845553d5e3f2f3
--- /dev/null
+++ b/lldb/test/API/commands/dwim-print/objc/Makefile
@@ -0,0 +1,3 @@
+OBJC_SOURCES := main.m
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/dwim-print/objc/TestDWIMPrintObjC.py b/lldb/test/API/commands/dwim-print/objc/TestDWIMPrintObjC.py
new file mode 100644
index 00000000000000..a642c40aeb7e4f
--- /dev/null
+++ b/lldb/test/API/commands/dwim-print/objc/TestDWIMPrintObjC.py
@@ -0,0 +1,16 @@
+"""
+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):
+
+ def test(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m"))
+ self.expect("dwim-print obj", substrs=["_number = 15"])
diff --git a/lldb/test/API/commands/dwim-print/objc/main.m b/lldb/test/API/commands/dwim-print/objc/main.m
new file mode 100644
index 00000000000000..5fdeb003238014
--- /dev/null
+++ b/lldb/test/API/commands/dwim-print/objc/main.m
@@ -0,0 +1,15 @@
+#import <Foundation/Foundation.h>
+
+@interface Object : NSObject
+@property(nonatomic) int number;
+@end
+
+@implementation Object
+@end
+
+int main(int argc, char **argv) {
+ Object *obj = [Object new];
+ obj.number = 15;
+ puts("break here");
+ return 0;
+}
|
|
✅ With the latest revision this PR passed the Python code formatter. |
| @@ -87,7 +87,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, | |||
|
|
|||
| DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions( | |||
| m_expr_options.m_verbosity, m_format_options.GetFormat()); | |||
| dump_options.SetHideRootName(suppress_result); | |||
| dump_options.SetHideRootName(suppress_result) | |||
| .SetExpandPointerTypeFlags(lldb::eTypeIsObjC); | |||
There was a problem hiding this comment.
Is this something we would want for frame var/expr too? In which case we could just check for ObjC in ValueObjectPrinter::ShouldPrintChildren directly instead of introducing this new API
There was a problem hiding this comment.
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.
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 to frame var/expr too.
There was a problem hiding this comment.
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.
When printing an ObjC object, which is a pointer, lldb has handled it like it treats any other pointer, printing only pointer address and class name. The object is not expanded, and its children are not shown.
This change updates the dwim-print to print expanded objc pointers - it is assumed that's what the user meant.
Note that this is currently possible using the
--ptr-depth/-Pflag, but the default is 0. With this change, when dwim-print prints root level objc objects, the default is effectively changed to 1.