JDevlieghere added inline comments.
================ Comment at: lldb/include/lldb/Target/StackFrameList.h:139-144 /// The currently selected frame. uint32_t m_selected_frame_idx; + // Records whether anyone has set the selected frame on this stack yet. + // We only let recognizers change the frame if this is the first time + // GetSelectedFrame is called. + bool m_selected_frame_set = false; ---------------- The purpose of the boolean is to differentiate between `m_selected_frame_idx == 0` meaning the zeroth frame is selected versus we haven't yet selected the current frame, right? If so I think we should make this an `std::optional<bool>`. ================ Comment at: lldb/source/Target/StackFrameList.cpp:823 + SelectMostRelevantFrame(); + m_selected_frame_set = true; + } ---------------- IIUC `SetSelectedFrame` always sets `m_selected_frame_set` to `true`, so is this redundant? ================ Comment at: lldb/source/Target/StackFrameList.cpp:883 m_concrete_frames_fetched = 0; + m_selected_frame_set = false; } ---------------- Should `m_selected_frame_idx` also be set to `0` here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147753/new/ https://reviews.llvm.org/D147753 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits