[lldb] Unify printing of internal and user stop hooks

Unify the printing of internal and user
(non-internal) stop hooks to make it more similar
to how we treat it for breakpoints.

This means replacing the `GetNumStopHooks()` and
`GetStopHookAtIndex()` with a copy of the stop
hooks (in C++20, we can change this to a constant
view). These functions were ever only used to
iterate the entire collection of stop hooks and
the implementation of `GetStopHookAtIndex()` turns
this into a `N^2` operation.

This change also allows us to cleanup a bit of
code at the call site.

Author: yln

lldb/include/lldb/Target/Target.h

@@ -1552,23 +1552,7 @@ class Target : public std::enable_shared_from_this<Target>,
 
   void SetAllStopHooksActiveState(bool active_state);
 
-  const std::vector<StopHookSP> &GetInternalStopHooks() const {
-    return m_internal_stop_hooks;
-  }
-
-  size_t GetNumStopHooks() const { return m_stop_hooks.size(); }
-
-  StopHookSP GetStopHookAtIndex(size_t index) {
-    if (index >= GetNumStopHooks())
-      return StopHookSP();
-    StopHookCollection::iterator pos = m_stop_hooks.begin();
-
-    while (index > 0) {
-      pos++;
-      index--;
-    }
-    return (*pos).second;
-  }
+  const std::vector<StopHookSP> GetStopHooks(bool internal = false) const;
 
   lldb::PlatformSP GetPlatform() { return m_platform_sp; }
 

lldb/source/Commands/CommandCompletions.cpp

@@ -777,13 +777,11 @@ void CommandCompletions::StopHookIDs(CommandInterpreter &interpreter,
   if (!target_sp)
     return;
 
-  const size_t num = target_sp->GetNumStopHooks();
-  for (size_t idx = 0; idx < num; ++idx) {
+  for (auto &stophook_sp : target_sp->GetStopHooks()) {
     StreamString strm;
     // The value 11 is an offset to make the completion description looks
     // neater.
     strm.SetIndentLevel(11);
-    const Target::StopHookSP stophook_sp = target_sp->GetStopHookAtIndex(idx);
     stophook_sp->GetDescription(strm, lldb::eDescriptionLevelInitial);
     request.TryCompleteCurrentArg(std::to_string(stophook_sp->GetID()),
                                   strm.GetString());

lldb/source/Commands/CommandObjectTarget.cpp

@@ -5273,27 +5273,17 @@ class CommandObjectTargetStopHookList : public CommandObjectParsed {
   void DoExecute(Args &command, CommandReturnObject &result) override {
     Target &target = GetTarget();
 
-    if (m_options.m_internal) {
-      for (auto &hook : target.GetInternalStopHooks()) {
-        hook->GetDescription(result.GetOutputStream(), eDescriptionLevelFull);
+    bool printed_hook = false;
+    for (auto &hook : target.GetStopHooks(m_options.m_internal)) {
+      if (printed_hook)
         result.GetOutputStream().PutCString("\n");
-      }
-      result.SetStatus(eReturnStatusSuccessFinishResult);
-      return;
+      hook->GetDescription(result.GetOutputStream(), eDescriptionLevelFull);
+      printed_hook = true;
     }
 
-    size_t num_hooks = target.GetNumStopHooks();
-    if (num_hooks == 0) {
+    if (!printed_hook)
       result.GetOutputStream().PutCString("No stop hooks.\n");
-    } else {
-      for (size_t i = 0; i < num_hooks; i++) {
-        Target::StopHookSP this_hook = target.GetStopHookAtIndex(i);
-        if (i > 0)
-          result.GetOutputStream().PutCString("\n");
-        this_hook->GetDescription(result.GetOutputStream(),
-                                  eDescriptionLevelFull);
-      }
-    }
+
     result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 

lldb/source/Target/Target.cpp

@@ -3107,6 +3107,23 @@ void Target::SetAllStopHooksActiveState(bool active_state) {
   }
 }
 
+// FIXME:  Ideally we would like to return a `const &` (const reference) instead
+//   of creating copy here, but that is not possible due to different container
+//   types.  In C++20, we should be able to use `std::ranges::views::values` to
+//   adapt the key-pair entries in the `std::map` (behind `StopHookCollection`)
+//   to avoid creating the copy.
+const std::vector<Target::StopHookSP>
+Target::GetStopHooks(bool internal) const {
+  if (internal)
+    return m_internal_stop_hooks;
+
+  std::vector<StopHookSP> stop_hooks;
+  for (auto &[_, hook] : m_stop_hooks)
+    stop_hooks.push_back(hook);
+
+  return stop_hooks;
+}
+
 bool Target::RunStopHooks(bool at_initial_stop) {
   if (m_suppress_stop_hooks)
     return false;