[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits that don't require re-review if they're fixed or turn out to be not relevant.



================
Comment at: lldb/include/lldb/Interpreter/ScriptedInterface.h:43-51
+  bool CheckStructuredDataObject(llvm::StringRef caller, T obj, Status &error) {
+    if (!obj || !obj->IsValid() || error.Fail()) {
+      return ErrorWithMessage<bool>(caller,
+                                    llvm::Twine("Null or invalid object (" +
+                                                llvm::Twine(error.AsCString()) +
+                                                llvm::Twine(")."))
+                                        .str(),
----------------
I think I left a similar comment in the past, but since you know whether the object is null or invalid, why drop that information and be less precise in your error message? 


================
Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:186
+void ScriptedThread::RefreshStateAfterStop() {
+  // TODO: Implement
+  if (m_reg_context_sp)
----------------
Still relevant?


================
Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:205-206
+        *reg_info, m_scripted_process.GetTarget().GetArchitecture());
+    assert(m_register_info_sp->GetNumRegisters() > 0);
+    assert(m_register_info_sp->GetNumRegisterSets() > 0);
+  }
----------------
Does this assertion depend on "user-input"? In other words, can this be triggered by not returning any registers from the interface? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585