[lldb] Display breakpoint locations using display name #90297
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a `show_function_display_name` parameter to `SymbolContext::DumpStopContext`. This parameter defaults to true, but `BreakpointLocation::GetDescription` sets it to true.
|
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesAdds a Full diff: https://github.com/llvm/llvm-project/pull/90297.diff 6 Files Affected:
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h
index bd33a71b46cac0..0bc707070f8504 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -158,6 +158,7 @@ class SymbolContext {
Stream *s, ExecutionContextScope *exe_scope, const Address &so_addr,
bool show_fullpaths, bool show_module, bool show_inlined_frames,
bool show_function_arguments, bool show_function_name,
+ bool show_function_display_name = false,
std::optional<Stream::HighlightSettings> settings = std::nullopt) const;
/// Get the address range contained within a symbol context.
diff --git a/lldb/include/lldb/Target/Language.h b/lldb/include/lldb/Target/Language.h
index 67714e6fdf942e..ff7c60bf68bfc9 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -281,6 +281,10 @@ class Language : public PluginInterface {
return mangled.GetMangledName();
}
+ virtual ConstString GetDisplayDemangledName(Mangled mangled) const {
+ return mangled.GetDemangledName();
+ }
+
virtual void GetExceptionResolverDescription(bool catch_on, bool throw_on,
Stream &s);
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index b48ec1398d63e8..41911fad41c648 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -507,7 +507,7 @@ void BreakpointLocation::GetDescription(Stream *s,
else
s->PutCString("where = ");
sc.DumpStopContext(s, m_owner.GetTarget().GetProcessSP().get(), m_address,
- false, true, false, true, true);
+ false, true, false, true, true, true);
} else {
if (sc.module_sp) {
s->EOL();
diff --git a/lldb/source/Core/Address.cpp b/lldb/source/Core/Address.cpp
index b23398883fa553..5a4751bd5256eb 100644
--- a/lldb/source/Core/Address.cpp
+++ b/lldb/source/Core/Address.cpp
@@ -645,7 +645,8 @@ bool Address::Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
pointer_sc.symbol != nullptr) {
s->PutCString(": ");
pointer_sc.DumpStopContext(s, exe_scope, so_addr, true, false,
- false, true, true, settings);
+ false, true, true, false,
+ settings);
}
}
}
@@ -685,7 +686,7 @@ bool Address::Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style,
sc.DumpStopContext(s, exe_scope, *this, show_fullpaths,
show_module, show_inlined_frames,
show_function_arguments, show_function_name,
- settings);
+ false, settings);
} else {
// We found a symbol but it was in a different section so it
// isn't the symbol we should be showing, just show the section
diff --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index b167c51fdce247..8efc4c639cca5f 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -310,6 +310,8 @@ ConstString Mangled::GetDemangledName() const {
}
ConstString Mangled::GetDisplayDemangledName() const {
+ if (Language *lang = Language::FindPlugin(GuessLanguage()))
+ return lang->GetDisplayDemangledName(*this);
return GetDemangledName();
}
diff --git a/lldb/source/Symbol/SymbolContext.cpp b/lldb/source/Symbol/SymbolContext.cpp
index f368896fbad490..8f26e41d192044 100644
--- a/lldb/source/Symbol/SymbolContext.cpp
+++ b/lldb/source/Symbol/SymbolContext.cpp
@@ -73,6 +73,7 @@ bool SymbolContext::DumpStopContext(
Stream *s, ExecutionContextScope *exe_scope, const Address &addr,
bool show_fullpaths, bool show_module, bool show_inlined_frames,
bool show_function_arguments, bool show_function_name,
+ bool show_function_display_name,
std::optional<Stream::HighlightSettings> settings) const {
bool dumped_something = false;
if (show_module && module_sp) {
@@ -93,6 +94,8 @@ bool SymbolContext::DumpStopContext(
ConstString name;
if (!show_function_arguments)
name = function->GetNameNoArguments();
+ if (!name && show_function_display_name)
+ name = function->GetDisplayName();
if (!name)
name = function->GetName();
if (name)
@@ -146,7 +149,8 @@ bool SymbolContext::DumpStopContext(
const bool show_function_name = true;
return inline_parent_sc.DumpStopContext(
s, exe_scope, inline_parent_addr, show_fullpaths, show_module,
- show_inlined_frames, show_function_arguments, show_function_name);
+ show_inlined_frames, show_function_arguments, show_function_name,
+ show_function_display_name);
}
} else {
if (line_entry.IsValid()) {
@@ -164,7 +168,12 @@ bool SymbolContext::DumpStopContext(
dumped_something = true;
if (symbol->GetType() == eSymbolTypeTrampoline)
s->PutCString("symbol stub for: ");
- s->PutCStringColorHighlighted(symbol->GetName().GetStringRef(), settings);
+ ConstString name;
+ if (show_function_display_name)
+ name = symbol->GetDisplayName();
+ if (!name)
+ name = symbol->GetName();
+ s->PutCStringColorHighlighted(name.GetStringRef(), settings);
}
if (addr.IsValid() && symbol->ValueIsAddress()) {
|
|
At the time of creation, this PR includes the changes in #90294. |
jimingham
approved these changes
Apr 26, 2024
|
This is just a suggestion, but would it make sense to interpret the client-facing "Language::GetDisplayName" to mean "get the name the user wants me to display", which is usually the language's display name (if it has one) but if there's no language display name or the caller has set the "get the full name" setting return GetName. The advantage of that interpretation is that anywhere you wanted to show the user a demangled name, you could JUST call GetDisplayName, rather than having to do "if (GetDisplayName) else GetName" in client code. And it makes sense, the user would never want to have nothing displayed if we had a name stashed somewhere... |
|
I think your commit summary has a typo. It should say that the parameter defaults to false, not true. |
| @@ -685,7 +686,7 @@ bool Address::Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle style, | |||
| sc.DumpStopContext(s, exe_scope, *this, show_fullpaths, | |||
| show_module, show_inlined_frames, | |||
| show_function_arguments, show_function_name, | |||
| settings); | |||
| false, settings); | |||
There was a problem hiding this comment.
Why is this one false?
There was a problem hiding this comment.
I wanted to limit the initial scope of changes to just breakpoint locations. It may be that we want to change this to true, but it has not be discussed (or audited, or tested) and so I am not confident in changing this too.
|
@jimingham I'm not sure I've understood your comment entirely. Are you saying the following?
What about the cases where the context ("is the name being displayed?") is unclear at the call site? This is why I introduced the |
adrian-prantl
approved these changes
May 2, 2024
kastiglione
deleted the
lldb-Display-breakpoint-locations-using-display-name
branch
kastiglione
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 9, 2024
Adds a `show_function_display_name` parameter to `SymbolContext::DumpStopContext`. This parameter defaults to false, but `BreakpointLocation::GetDescription` sets it to true. This is NFC in mainline lldb, and will be used to modify how Swift breakpoint locations are printed. (cherry picked from commit 7ec8a33)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a
show_function_display_nameparameter toSymbolContext::DumpStopContext. Thisparameter defaults to false, but
BreakpointLocation::GetDescriptionsets it to true.This is NFC in mainline lldb, and will be used to modify how Swift breakpoint locations are printed.