[Lldb-commits] [PATCH] D97739: Add a progress class that can track and report long running operations that happen in LLDB.

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked 4 inline comments as done.
clayborg added inline comments.


================
Comment at: lldb/source/API/SBDebugger.cpp:857
+        error = m_opaque_sp->GetTargetList().CreateTarget(
+            *m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp,
+            target_sp);
----------------
I submit with "arc diff" and it will cause lint errors if I don't allow it to fix the lint errors it finds.


================
Comment at: lldb/source/Core/Debugger.cpp:1188
+        (*pos)->ReportProgressPrivate(progress_id, message, completed, total,
+                                      is_debugger_specific);
+    }
----------------
In the Progress class I currently only store a pointer to the debugger if one was specified. This means there is a lifetime issue that can arise. If I do the callback for Debugger::ReportProgress(...) above, then we will only notify debugger instances that are still around and in the global list. If not then we need to either store a DebuggerSP or DebuggerWP in the Progress object, which we can do if truly needed, but I don't like DebuggerSP as I don't want a Progress to keep a debugger around. DebuggerWP is possible if we really need this. Let me know what you think. Debugger::ReportProgressPrivate() could be make into a static function if needed so that no one sees it from Debugger.h.


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1898-1900
+                          "error: unable to find section %d for a symbol in "
+                          "%s, corrupt file?\n",
+                          n_sect, filename.c_str());
----------------
again, I use "arc diff" which must fix lint issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97739/new/

https://reviews.llvm.org/D97739