Skip to content

[lldb] Avoid unnecessary regex check in dwim-print #114608

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.

Sign up for GitHub

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
merged 1 commit into from
Mar 8, 2025
Merged

[lldb] Avoid unnecessary regex check in dwim-print #114608

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions lldb/source/Commands/CommandObjectDWIMPrint.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// Add a hint if object description was requested, but no description
// function was implemented.
auto maybe_add_hint = [&](llvm::StringRef output) {
static bool note_shown = false;
if (note_shown)
return;
Comment on lines +104 to +106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this code was already there, but this isn't great. Can we store this at the debugger level, for example? I'd expect this to show up once per debugger, and again after having called SBDebugger::Terminate/Initialize.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we tie it to a module, we could use the mechanism introduced in https://github.com/llvm/llvm-project/pull/112801/files to automatically show it once per session.

I'd expect this to show up once per debugger

You mean once per type and debugger, right?

Copy link
Contributor Author

@kastiglione kastiglione Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @augusto2112 and I discussed and agreed not to do once per type. We figured that might make the hint too noisy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am indifferent about per-debugger. I don't consider this note as being critical, as long as it's shown once in a while, I'm good.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we already landed on only once per session I'm fine with that too.
You can also manually allocate a std::once and pass it to the Debugger-wide function. Or just do the call_once here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the idea was for it not to show up too frequently, I'd be ok if it's once per debugger too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how call_once would work here. This lambda might be called once, or it might be called multiple times (even indefinitely).


// Identify the default output of object description for Swift and
// Objective-C
// "<Name: 0x...>. The regex is:
Expand All @@ -110,17 +114,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
// - 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*$");
static 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.AppendNote(
"object description requested, but type doesn't implement "
"a custom object description. Consider using \"p\" instead of "
Expand Down