augusto2112 updated this revision to Diff 546275.
augusto2112 added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153489/new/

https://reviews.llvm.org/D153489

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/test/API/lang/objc/objc-po-hint/Makefile
  lldb/test/API/lang/objc/objc-po-hint/TestObjcPoHint.py
  lldb/test/API/lang/objc/objc-po-hint/main.m

Index: lldb/test/API/lang/objc/objc-po-hint/main.m
===================================================================
--- /dev/null
+++ lldb/test/API/lang/objc/objc-po-hint/main.m
@@ -0,0 +1,22 @@
+#import <Foundation/Foundation.h>
+
+@interface Foo : NSObject {}
+
+-(id) init;
+
+@end
+
+@implementation Foo
+
+-(id) init
+{
+    return self = [super init];
+}
+@end
+
+int main()
+{
+    Foo *foo = [Foo new];
+    NSLog(@"a"); // Set breakpoint here.
+    return 0;
+}
Index: lldb/test/API/lang/objc/objc-po-hint/TestObjcPoHint.py
===================================================================
--- /dev/null
+++ lldb/test/API/lang/objc/objc-po-hint/TestObjcPoHint.py
@@ -0,0 +1,49 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestObjcPoHint(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_show_po_hint(self):
+        ### Test that the po hint is shown once with the DWIM print command
+        self.build()
+        _, _, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "Set breakpoint here", lldb.SBFileSpec("main.m")
+        )
+        # Make sure the hint is printed the first time
+        self.expect(
+            "dwim-print -O -- foo",
+            substrs=[
+                "note: object description requested, but type doesn't implement "
+                'a custom object description. Consider using "p" instead of "po"',
+                "<Foo: 0x",
+            ],
+        )
+
+        # Make sure it's not printed again.
+        self.expect(
+            "dwim-print -O -- foo",
+            substrs=[
+                "note: object description"
+            ],
+            matching=False,
+        )
+
+    def test_show_po_hint_disabled(self):
+        ### Test that when the setting is disabled the hint is not printed
+        self.build()
+        _, _, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "Set breakpoint here", lldb.SBFileSpec("main.m")
+        )
+        self.runCmd("setting set show-dont-use-po-hint false")
+        # Make sure the hint is printed the first time
+        self.expect(
+            "dwim-print -O -- foo",
+            substrs=[
+                "note: object description"
+            ],
+            matching=False,
+        )
Index: lldb/test/API/lang/objc/objc-po-hint/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/lang/objc/objc-po-hint/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS = -lobjc -framework Foundation
+
+include Makefile.rules
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -434,6 +434,12 @@
       idx, g_debugger_properties[idx].default_cstr_value);
 }
 
+bool Debugger::GetShowDontUsePoHint() const {
+  const uint32_t idx = ePropertyShowDontUsePoHint;  
+  return GetPropertyAtIndexAs<bool>(
+      idx, g_debugger_properties[idx].default_uint_value != 0);
+}
+
 bool Debugger::GetUseSourceCache() const {
   const uint32_t idx = ePropertyUseSourceCache;
   return GetPropertyAtIndexAs<bool>(
Index: lldb/source/Core/CoreProperties.td
===================================================================
--- lldb/source/Core/CoreProperties.td
+++ lldb/source/Core/CoreProperties.td
@@ -195,6 +195,10 @@
     Global,
     DefaultStringValue<"${ansi.normal}">,
     Desc<"When displaying suggestion in a color-enabled terminal, use the ANSI terminal code specified in this format immediately after the suggestion.">;
+  def ShowDontUsePoHint: Property<"show-dont-use-po-hint", "Boolean">,
+    Global,
+    DefaultTrue,
+    Desc<"If true, and object description was requested for a type that does not implement it, LLDB will print a hint telling the user to consider using p instead.">;
   def DWIMPrintVerbosity: Property<"dwim-print-verbosity", "Enum">,
     Global,
     DefaultEnumValue<"eDWIMPrintVerbosityNone">,
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===================================================================
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -25,6 +25,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatVariadic.h"
 
+#include <regex>
+
 using namespace llvm;
 using namespace lldb;
 using namespace lldb_private;
@@ -95,8 +97,46 @@
       m_expr_options.m_verbosity, m_format_options.GetFormat());
   dump_options.SetHideRootName(suppress_result);
 
+  bool is_po = m_varobj_options.use_objc;
+
   StackFrame *frame = m_exe_ctx.GetFramePtr();
 
+  // Either Swift was explicitly specified, or the frame is Swift.
+  lldb::LanguageType language = m_expr_options.language;
+  if (language == lldb::eLanguageTypeUnknown && frame)
+    language = frame->GuessLanguage();
+
+  // Add a hint if object description was requested, but no description
+  // function was implemented.
+  auto maybe_add_hint = [&](llvm::StringRef output) {
+    // Identify the default output of object description for Swift and
+    // Objective-C
+    // "<Name: 0x...>. The regex is:
+    // - Start with "<".
+    // - Followed by 1 or more non-whitespace characters.
+    // - Followed by ": 0x".
+    // - Followed by 5 or more hex digits.
+    // - Followed by ">".
+    // - End with zero or more whitespace characters.
+    const std::regex swift_class_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
+
+    if (GetDebugger().GetShowDontUsePoHint() && target_ptr &&
+        (language == lldb::eLanguageTypeSwift ||
+         language == lldb::eLanguageTypeObjC) &&
+        std::regex_match(output.data(), swift_class_regex)) {
+
+      static bool note_shown = false;
+      if (note_shown)
+        return;
+
+      result.GetOutputStream()
+          << "note: object description requested, but type doesn't implement "
+             "a custom object description. Consider using \"p\" instead of "
+             "\"po\" (this note will only be shown once per debug session).\n";
+      note_shown = true;
+    }
+  };
+
   // First, try `expr` as the name of a frame variable.
   if (frame) {
     auto valobj_sp = frame->FindVariable(ConstString(expr));
@@ -114,7 +154,15 @@
                                         flags, expr);
       }
 
-      valobj_sp->Dump(result.GetOutputStream(), dump_options);
+      if (is_po) {
+        StreamString temp_result_stream;
+        valobj_sp->Dump(temp_result_stream, dump_options);
+        llvm::StringRef output = temp_result_stream.GetString();
+        maybe_add_hint(output);
+        result.GetOutputStream() << output;
+      } else {
+        valobj_sp->Dump(result.GetOutputStream(), dump_options);
+      }
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
@@ -135,8 +183,17 @@
                                         expr);
       }
 
-      if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
-        valobj_sp->Dump(result.GetOutputStream(), dump_options);
+      if (valobj_sp->GetError().GetError() != UserExpression::kNoResult) {
+        if (is_po) {
+          StreamString temp_result_stream;
+          valobj_sp->Dump(temp_result_stream, dump_options);
+          llvm::StringRef output = temp_result_stream.GetString();
+          maybe_add_hint(output);
+          result.GetOutputStream() << output;
+        } else {
+          valobj_sp->Dump(result.GetOutputStream(), dump_options);
+        }
+      }
 
       if (suppress_result)
         if (auto result_var_sp =
Index: lldb/include/lldb/Core/Debugger.h
===================================================================
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -321,6 +321,8 @@
 
   llvm::StringRef GetAutosuggestionAnsiSuffix() const;
 
+  bool GetShowDontUsePoHint() const;
+
   bool GetUseSourceCache() const;
 
   bool SetUseSourceCache(bool use_source_cache);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to