[lldb] Update dwim-print to show expanded objc instances #117500

kastiglione · 2024-11-24T18:23:21Z

When printing an ObjC object, which is a pointer, lldb has handled it the same way it treats any other pointer – printing only class name and pointer address. The object is not expanded, its children are not shown.

This change updates dwim-print to print objc pointers by expanding (ie dereferencing), with the assumption that it's what the user wants.

Note that this is currently possible using the --ptr-depth/-P flag. With this change, when dwim-print prints root level objc objects, it's the same effect as using --ptr-depth 1.

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.

llvmbot · 2024-11-24T18:23:56Z

@llvm/pr-subscribers-lldb

Author: Dave Lee (kastiglione)

Changes

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.

Full diff: https://github.com/llvm/llvm-project/pull/117500.diff

7 Files Affected:

(modified) lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h (+3)
(modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+2-1)
(modified) lldb/source/DataFormatters/DumpValueObjectOptions.cpp (+6)
(modified) lldb/source/DataFormatters/ValueObjectPrinter.cpp (+8-6)
(added) lldb/test/API/commands/dwim-print/objc/Makefile (+3)
(added) lldb/test/API/commands/dwim-print/objc/TestDWIMPrintObjC.py (+16)
(added) lldb/test/API/commands/dwim-print/objc/main.m (+15)

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;
+}

github-actions · 2024-11-24T18:28:14Z

✅ With the latest revision this PR passed the Python code formatter.

Michael137 · 2024-11-27T11:20:43Z

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);


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

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.

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.

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.

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

Should/Could this perhaps be an (NSObject *) data formatter, doing this work, so it's applied consistently?

@adrian-prantl I tried this (with a Python implementation) and that didn't work for two reasons:

Setting a synthetic for NSObject * did not result in that formatter being used with Subclass *

An objc pointer is still not expanded, thus the production of children doesn't get triggered unless I explicitly expand the pointer with v -P1 <variable>

adrian-prantl

I think this is useful.

kastiglione · 2025-03-12T23:51:33Z

@jimingham this is the PR I mentioned in person on Tuesday.

jimingham · 2025-03-13T21:03:18Z

If the result of frame var is 30 pointer variables, I would not be happy if that went from 30 lines I could parse and find the variable I was interested in to 30 x num_ivars lines, and now I've got many pages of much more noisy output to scan.

When I'm listing multiple variables, I'm doing so because I want to be able to scan them in totality, not dive into one of them. I definitely would not want this feature when more than one top level variable was being printed. For the same reason, I would definitely not want to do this for any contained objects, just top level vars.

It would be less disruptive if we did this when printing only ONE variable. It would be weird to have v the children in v array_of_nsobjects look quite different from v foo shows? The collection one would be fairly easy to understand, the difference between v and v foo is a little odder, though people could get used to this.

We also have to make sure that we don't end up expanding variables that people have summaries for, and consequently don't need or want to see the children. Type summaries currently have --expand off by default, and we do want to preserve that behavior.

kastiglione · 2025-03-14T04:51:21Z

@jimingham I agree that expanding the output of v while listing variables could be a regression. However note that this change is to dwim-print, not v, and so this change would not cause the problem you're describing. This is because dwim-print has neither a "print all variables", nor a "print more than one variable" mode.

In the discussion above @Michael137 asked whether v and expr should also have this expand behavior, and you've given a good argument for v to not have this behavior.

We also have to make sure that we don't end up expanding variables that people have summaries for

I hadn't thought about this. I'll add a test for this case.

Other than ensuring objects are not expanded when their summary dictates they not be expanded, are you ok with this change to dwim-print?

jimingham · 2025-03-14T18:51:23Z

Yes, if this is for dwim-print/expr and only -P1, and obeys the summary dictates, this is fine.

When printing an ObjC object, which is a pointer, lldb has handled it the same way it treats any other pointer – printing only class name and pointer address. The object is not expanded, its children are not shown. This change updates `dwim-print` to print objc pointers by expanding (ie dereferencing), with the assumption that it's what the user wants. Note that this is currently possible using the `--ptr-depth`/`-P` flag. With this change, when `dwim-print` prints root level objc objects, it's the same effect as using `--ptr-depth 1`. (cherry picked from commit 65e68a3)

kastiglione requested a review from JDevlieghere as a code owner November 24, 2024 18:23

llvmbot added the lldb label Nov 24, 2024

kastiglione requested review from adrian-prantl and jimingham November 24, 2024 18:23

Michael137 reviewed Nov 27, 2024

View reviewed changes

kastiglione added 2 commits December 2, 2024 16:29

python formatting

1890aaa

Apply skipUnlessDarwin decorator

40aadad

adrian-prantl approved these changes Mar 12, 2025

View reviewed changes

Test expansion of nested objc objects

668e91d

Test for no expansion when summary formatter is present

0e8ddf0

kastiglione merged commit 65e68a3 into llvm:main Mar 15, 2025
10 checks passed

kastiglione deleted the lldb-Update-dwim-print-to-show-expanded-objc-instances branch March 15, 2025 15:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb] Update dwim-print to show expanded objc instances #117500

[lldb] Update dwim-print to show expanded objc instances #117500

Uh oh!

kastiglione commented Nov 24, 2024 •

edited

Loading

Uh oh!

llvmbot commented Nov 24, 2024

Uh oh!

github-actions bot commented Nov 24, 2024 •

edited

Loading

Uh oh!

Michael137 Nov 27, 2024 •

edited

Loading

Uh oh!

kastiglione Nov 27, 2024

Uh oh!

kastiglione Nov 28, 2024

Uh oh!

Michael137 Nov 28, 2024 •

edited

Loading

Uh oh!

adrian-prantl Dec 3, 2024

Uh oh!

kastiglione Mar 11, 2025

Uh oh!

adrian-prantl left a comment

Uh oh!

kastiglione commented Mar 12, 2025

Uh oh!

jimingham commented Mar 13, 2025 •

edited

Loading

Uh oh!

kastiglione commented Mar 14, 2025

Uh oh!

jimingham commented Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

[lldb] Update dwim-print to show expanded objc instances #117500

[lldb] Update dwim-print to show expanded objc instances #117500

Uh oh!

Conversation

kastiglione commented Nov 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 24, 2024

Uh oh!

github-actions bot commented Nov 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael137 Nov 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kastiglione Nov 27, 2024

Choose a reason for hiding this comment

Uh oh!

kastiglione Nov 28, 2024

Choose a reason for hiding this comment

Uh oh!

Michael137 Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrian-prantl Dec 3, 2024

Choose a reason for hiding this comment

Uh oh!

kastiglione Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

adrian-prantl left a comment

Choose a reason for hiding this comment

Uh oh!

kastiglione commented Mar 12, 2025

Uh oh!

jimingham commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kastiglione commented Mar 14, 2025

Uh oh!

jimingham commented Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

kastiglione commented Nov 24, 2024 •

edited

Loading

github-actions bot commented Nov 24, 2024 •

edited

Loading

Michael137 Nov 27, 2024 •

edited

Loading

Michael137 Nov 28, 2024 •

edited

Loading

jimingham commented Mar 13, 2025 •

edited

Loading