[Lldb-commits] [lldb] [lldb] Fix DIL error diagnostics output (PR #187680)

[Ilia Kuklin via lldb-commits]

================
@@ -637,8 +637,21 @@ may even involve JITing and running code in the target program.)");
               Stream &output_stream = result.GetOutputStream();
               options.SetRootValueObjectName(
                   valobj_sp->GetParent() ? entry.c_str() : nullptr);
-              if (llvm::Error error = valobj_sp->Dump(output_stream, options))
-                result.AppendError(toString(std::move(error)));
+              // Check only the `error` argument, because doing
+              // `valobj_sp->GetError()` will update the value and potentially
+              // return a new error that happens during the update, even if
+              // `GetValueForVariableExpressionPath` reported no errors.
----------------
kuilpd wrote:

Yes, if there is an error reported by DIL, we want it to be handled by `DiagnosticsRendering`. If no error, `Dump` the value. 

The comment basically warns not to use `valobj_sp->GetError()` to check the error here, because it updates the value and can generate a new simple string error without any diagnostic information for `DiagnosticsRendering`. That's why I cloned the error into `&error` argument in DIL, so that it can be retrieved without updating the value.

All this is to get this output for cases like this:
```
(lldb) v *((int*)0)
(int) *((int*)0) = <parent is NULL>
```
because the legacy `frame var` still exists and this is how it outputs the error, and the code in `CommandObjectFrame` doesn't know whether DIL was called or the legacy `frame var`. If we ever remove it, we can change this, probably by updating the final value in DIL itself before returning it, and creating a proper diagnostic error message.


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