augusto2112 updated this revision to Diff 535561.
augusto2112 marked 5 inline comments as done.
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/include/lldb/Target/Target.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):
+ 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 requested, but type doesn't implement "
+ 'a custom object description. Consider using "p" instead of "po"',
+ ],
+ 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 requested, but type doesn't implement "
+ 'a custom object description. Consider using "p" instead of "po"',
+ ],
+ 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
@@ -435,6 +435,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 llvm::SmallPtrSet<Target *, 4> targets_warned;
+ if (targets_warned.contains(target_ptr))
+ return;
+
+ result.GetOutputStream()
+ << "note: object description requested, but type doesn't implement "
+ "a custom object description. Consider using \"p\" instead of "
+ "\"po\"\n";
+ targets_warned.insert(target_ptr);
+ }
+ };
+
// 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/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -493,6 +493,7 @@
eBroadcastBitSymbolsChanged = (1 << 5),
};
+ std::once_flag target_once_flag;
// These two functions fill out the Broadcaster interface:
static ConstString &GetStaticBroadcasterClass();
Index: lldb/include/lldb/Core/Debugger.h
===================================================================
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -318,6 +318,8 @@
llvm::StringRef GetAutosuggestionAnsiSuffix() const;
+ bool GetShowDontUsePoHint() const;
+
bool GetUseSourceCache() const;
bool SetUseSourceCache(bool use_source_cache);
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
