-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb] add a marker before skipped frames #167550
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
533989a
73548e6
1ba6ded
2bd30fd
78385d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,6 +64,17 @@ void RenderDiagnosticDetails(Stream &stream, | |
| bool show_inline, | ||
| llvm::ArrayRef<DiagnosticDetail> details); | ||
|
|
||
| /// Returns whether or not the current terminal supports Unicode rendering. | ||
| /// | ||
| /// The value is cached after the first computation. | ||
| /// | ||
| /// On POSIX systems, we check if the LANG environment variable contains the | ||
| /// substring "UTF-8"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's so much better than checking for color! |
||
| /// | ||
| /// On Windows, we check that we are running from the Windows Terminal | ||
| /// application. | ||
| bool TerminalSupportsUnicode(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we move this into |
||
|
|
||
| class DiagnosticError | ||
| : public llvm::ErrorInfo<DiagnosticError, CloneableECError> { | ||
| public: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1935,7 +1935,7 @@ bool StackFrame::DumpUsingFormat(Stream &strm, | |
| } | ||
|
|
||
| void StackFrame::DumpUsingSettingsFormat(Stream *strm, bool show_unique, | ||
| const char *frame_marker) { | ||
| const std::string &frame_marker) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're passing this into a function that takes a StringRef — why not make it a StringRef, too? |
||
| if (strm == nullptr) | ||
| return; | ||
|
|
||
|
|
@@ -1953,6 +1953,7 @@ void StackFrame::DumpUsingSettingsFormat(Stream *strm, bool show_unique, | |
| frame_format = &format_entry; | ||
| } | ||
| } | ||
|
|
||
| if (!DumpUsingFormat(*strm, frame_format, frame_marker)) { | ||
| Dump(strm, true, false); | ||
| strm->EOL(); | ||
|
|
@@ -2034,7 +2035,7 @@ bool StackFrame::HasCachedData() const { | |
| } | ||
|
|
||
| bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source, | ||
| bool show_unique, const char *frame_marker) { | ||
| bool show_unique, const std::string &frame_marker) { | ||
| if (show_frame_info) { | ||
| strm.Indent(); | ||
| DumpUsingSettingsFormat(&strm, show_unique, frame_marker); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,9 +23,11 @@ | |
| #include "lldb/Target/Target.h" | ||
| #include "lldb/Target/Thread.h" | ||
| #include "lldb/Target/Unwind.h" | ||
| #include "lldb/Utility/DiagnosticsRendering.h" | ||
| #include "lldb/Utility/LLDBLog.h" | ||
| #include "lldb/Utility/Log.h" | ||
| #include "llvm/ADT/SmallPtrSet.h" | ||
| #include "llvm/Support/ConvertUTF.h" | ||
|
|
||
| #include <memory> | ||
|
|
||
|
|
@@ -879,11 +881,43 @@ StackFrameList::GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr) { | |
| return ret_sp; | ||
| } | ||
|
|
||
| bool StackFrameList::IsNextFrameHidden(lldb_private::StackFrame &frame) { | ||
| uint32_t frame_idx = frame.GetFrameIndex(); | ||
| StackFrameSP frame_sp = GetFrameAtIndex(frame_idx + 1); | ||
| if (!frame_sp) | ||
| return false; | ||
| return frame_sp->IsHidden(); | ||
| } | ||
|
|
||
| bool StackFrameList::IsPreviousFrameHidden(lldb_private::StackFrame &frame) { | ||
| uint32_t frame_idx = frame.GetFrameIndex(); | ||
| if (frame_idx == 0) | ||
| return false; | ||
| StackFrameSP frame_sp = GetFrameAtIndex(frame_idx - 1); | ||
| if (!frame_sp) | ||
| return false; | ||
| return frame_sp->IsHidden(); | ||
| } | ||
|
|
||
| std::wstring StackFrameList::FrameMarker(lldb::StackFrameSP frame_sp, | ||
| lldb::StackFrameSP selected_frame_sp) { | ||
| if (frame_sp == selected_frame_sp) { | ||
| return TerminalSupportsUnicode() ? L" * " : L"* "; | ||
| } else if (!TerminalSupportsUnicode()) { | ||
| return L" "; | ||
| } else if (IsPreviousFrameHidden(*frame_sp)) { | ||
| return L" ﹉"; | ||
| } else if (IsNextFrameHidden(*frame_sp)) { | ||
| return L" ﹍"; | ||
| } | ||
|
Comment on lines
+904
to
+912
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No braces |
||
| return L" "; | ||
| } | ||
|
|
||
| size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame, | ||
| uint32_t num_frames, bool show_frame_info, | ||
| uint32_t num_frames_with_source, | ||
| bool show_unique, bool show_hidden, | ||
| const char *selected_frame_marker) { | ||
| bool show_selected_frame) { | ||
| size_t num_frames_displayed = 0; | ||
|
|
||
| if (num_frames == 0) | ||
|
|
@@ -901,25 +935,17 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame, | |
|
|
||
| StackFrameSP selected_frame_sp = | ||
| m_thread.GetSelectedFrame(DoNoSelectMostRelevantFrame); | ||
| const char *unselected_marker = nullptr; | ||
| std::string buffer; | ||
| if (selected_frame_marker) { | ||
| size_t len = strlen(selected_frame_marker); | ||
| buffer.insert(buffer.begin(), len, ' '); | ||
| unselected_marker = buffer.c_str(); | ||
| } | ||
| const char *marker = nullptr; | ||
| std::wstring marker; | ||
| for (frame_idx = first_frame; frame_idx < last_frame; ++frame_idx) { | ||
| frame_sp = GetFrameAtIndex(frame_idx); | ||
| if (!frame_sp) | ||
| break; | ||
|
|
||
| if (selected_frame_marker != nullptr) { | ||
| if (frame_sp == selected_frame_sp) | ||
| marker = selected_frame_marker; | ||
| else | ||
| marker = unselected_marker; | ||
| } | ||
| if (show_selected_frame) | ||
| marker = FrameMarker(frame_sp, selected_frame_sp); | ||
| else | ||
| marker = FrameMarker(frame_sp, nullptr); | ||
|
|
||
| // Hide uninteresting frames unless it's the selected frame. | ||
| if (!show_hidden && frame_sp != selected_frame_sp && frame_sp->IsHidden()) | ||
|
|
@@ -933,10 +959,11 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame, | |
| m_thread.GetID(), num_frames_displayed)) | ||
| break; | ||
|
|
||
|
|
||
| std::string marker_utf8; | ||
| llvm::convertWideToUTF8(marker, marker_utf8); | ||
| if (!frame_sp->GetStatus(strm, show_frame_info, | ||
| num_frames_with_source > (first_frame - frame_idx), | ||
| show_unique, marker)) | ||
| show_unique, marker_utf8)) | ||
| break; | ||
| ++num_frames_displayed; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -102,7 +102,7 @@ void RenderDiagnosticDetails(Stream &stream, | |||||
| // characters. In the future it might make sense to move this into | ||||||
| // Host so it can be customized for a specific platform. | ||||||
| llvm::StringRef cursor, underline, vbar, joint, hbar, spacer; | ||||||
| if (stream.AsRawOstream().colors_enabled()) { | ||||||
| if (TerminalSupportsUnicode()) { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, this is long overdue! |
||||||
| cursor = "˄"; | ||||||
| underline = "˜"; | ||||||
| vbar = "│"; | ||||||
|
|
@@ -232,4 +232,19 @@ void RenderDiagnosticDetails(Stream &stream, | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| bool TerminalSupportsUnicode() { | ||||||
| static std::optional<bool> result; | ||||||
| if (result) | ||||||
| return result.value(); | ||||||
| #ifndef _WIN32 | ||||||
| if (const char *lang_var = std::getenv("LANG")) | ||||||
| result = std::string(lang_var).find("UTF-8"); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| else | ||||||
| result = false; | ||||||
| #else | ||||||
| result = std::getenv("WT_SESSION") != nullptr; | ||||||
| #endif | ||||||
| return result.value(); | ||||||
| } | ||||||
|
Comment on lines
+235
to
+248
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this be correct for Xcode and DAP? |
||||||
|
|
||||||
| } // namespace lldb_private | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StringRef?