[Lldb-commits] [PATCH] D135998: Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, kastiglione, labath.
Herald added a project: All.
aprantl requested review of this revision.

When `UserExpression::Evaluate()` fails and doesn't return a ValueObject there is no vehicle for returning the error in the return value.

This behavior can be observed by applying the following patch:

  diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
  index f1a311b7252c..58c03ccdb068 100644
  --- a/lldb/source/Target/Target.cpp
  +++ b/lldb/source/Target/Target.cpp
  @@ -2370,6 +2370,7 @@ UserExpression *Target::GetUserExpressionForLanguage(
       Expression::ResultType desired_type,
       const EvaluateExpressionOptions &options, ValueObject *ctx_obj,
       Status &error) {
  +  error.SetErrorStringWithFormat("Ha ha!");  return nullptr;
     auto type_system_or_err = GetScratchTypeSystemForLanguage(language);
     if (auto err = type_system_or_err.takeError()) {
       error.SetErrorStringWithFormat(

and then running

  $ lldb -o "p 1"
  (lldb) p 1
  (lldb)

This patch fixes this by creating an empty result ValueObject that wraps the error.


https://reviews.llvm.org/D135998

Files:
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -26,6 +26,7 @@
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObject.h"
+#include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Expression/DiagnosticManager.h"
 #include "lldb/Expression/ExpressionVariable.h"
 #include "lldb/Expression/REPL.h"
@@ -2528,6 +2529,10 @@
     execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
                                                  result_valobj_sp, error,
                                                  fixed_expression, ctx_obj);
+    // Pass up the error by wrapping it inside an error result.
+    if (error.Fail() && !result_valobj_sp)
+      result_valobj_sp = ValueObjectConstResult::Create(
+          exe_ctx.GetBestExecutionContextScope(), error);
   }
 
   if (execution_results == eExpressionCompleted)
Index: lldb/source/Commands/CommandObjectExpression.cpp
===================================================================
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -461,6 +461,8 @@
         result.SetStatus(eReturnStatusFailed);
       }
     }
+  } else {
+    error_stream.Printf("error: unknown error\n");
   }
 
   return (success != eExpressionSetupError &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135998.467965.patch
Type: text/x-patch
Size: 1457 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20221014/4a226a1e/attachment-0001.bin>