[Lldb-commits] [lldb] ValueImpl::GetSP was calling GetError before acquiring the StopLocker. (PR #225976)

[via lldb-commits]

================
@@ -3893,12 +3895,13 @@ lldb::ValueObjectSP ValueImpl::GetSP(Process::StopLocker &stop_locker,
   lldb::ValueObjectSP value_sp = m_valobj_sp;
 
   Target *target = value_sp->GetTargetSP().get();
-  // If this ValueObject holds an error, then it is valuable for that.
-  if (value_sp->GetError().Fail())
-    return value_sp;
 
-  if (!target)
+  if (!target) {
----------------
jimingham wrote:

I don't think that makes anything clearer.  If it's not safe to update, we want to figure out the error we can return (with PeekError()) and then not do any more work.  If we instead set the `safe_to_update` boolean, we'd then have to use it to decide whether to call PeekError or GetError, and then we'd have to check it again and return an error if it was not safe to update.

That makes it less clear that we need to not do any more work here if we can't get the locker.

https://github.com/llvm/llvm-project/pull/225976