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

Reply via email to