Make sure Target::EvaluateExpression() passes up an error instead of silently droppin...
authorAdrian Prantl <aprantl@apple.com>
Fri, 14 Oct 2022 23:45:01 +0000 (16:45 -0700)
committerAdrian Prantl <aprantl@apple.com>
Tue, 18 Oct 2022 00:27:54 +0000 (17:27 -0700)
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.

Differential Revision: https://reviews.llvm.org/D135998

lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Target/Target.cpp
lldb/test/API/commands/expression/context-object/TestContextObject.py
lldb/test/API/commands/expression/fixits/TestFixIts.py

index b7d129e..be306f2 100644 (file)
@@ -461,6 +461,8 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
         result.SetStatus(eReturnStatusFailed);
       }
     }
+  } else {
+    error_stream.Printf("error: unknown error\n");
   }
 
   return (success != eExpressionSetupError &&
index c567407..f1a311b 100644 (file)
@@ -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 @@ ExpressionResults Target::EvaluateExpression(
     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 45f7a00..7c963eb 100644 (file)
@@ -67,7 +67,7 @@ class ContextObjectTestCase(TestBase):
 
         # Test an expression evaluation
         value = obj_val.EvaluateExpression("1")
-        self.assertFalse(value.IsValid())
+        self.assertTrue(value.IsValid())
         self.assertFalse(value.GetError().Success())
 
         #
@@ -79,7 +79,7 @@ class ContextObjectTestCase(TestBase):
 
         # Test an expression evaluation
         value = obj_val.EvaluateExpression("1")
-        self.assertFalse(value.IsValid())
+        self.assertTrue(value.IsValid())
         self.assertFalse(value.GetError().Success())
 
         # Test retrieveing of an element's field
@@ -97,7 +97,7 @@ class ContextObjectTestCase(TestBase):
 
         # Test an expression evaluation
         value = obj_val.EvaluateExpression("1")
-        self.assertFalse(value.IsValid())
+        self.assertTrue(value.IsValid())
         self.assertFalse(value.GetError().Success())
 
         # Test retrieveing of a dereferenced object's field
@@ -127,7 +127,7 @@ class ContextObjectTestCase(TestBase):
 
         # Test an expression evaluation
         value = obj_val.EvaluateExpression("1")
-        self.assertFalse(value.IsValid())
+        self.assertTrue(value.IsValid())
         self.assertFalse(value.GetError().Success())
 
         # Test retrieveing of a dereferenced object's field
index bcc7e61..cd82a2f 100644 (file)
@@ -47,10 +47,11 @@ class ExprCommandWithFixits(TestBase):
         # The Fix-It changes "ptr.m" to "ptr->m".
         expr = "struct MyTy { int m; }; MyTy x; MyTy *ptr = &x; int m = ptr.m;"
         value = frame.EvaluateExpression(expr, top_level_options)
-        # A successfully parsed top-level expression will yield an error
-        # that there is 'no value'. If a parsing error would have happened we
-        # would get a different error kind, so let's check the error kind here.
-        self.assertEquals(value.GetError().GetCString(), "error: No value")
+        # A successfully parsed top-level expression will yield an
+        # unknown error . If a parsing error would have happened we
+        # would get a different error kind, so let's check the error
+        # kind here.
+        self.assertEquals(value.GetError().GetCString(), "unknown error")
 
         # Try with two errors:
         two_error_expression = "my_pointer.second->a"
@@ -170,7 +171,7 @@ class ExprCommandWithFixits(TestBase):
         multiple_runs_options.SetRetriesWithFixIts(2)
         value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
         # This error signals success for top level expressions.
-        self.assertEquals(value.GetError().GetCString(), "error: No value")
+        self.assertEquals(value.GetError().GetCString(), "unknown error")
 
         # Test that the code above compiles to the right thing.
         self.expect_expr("test_X(1)", result_type="int", result_value="123")