Return a useful "Error" for an expression that completes but produces no result

[jingham]

Summary

When you run an expression like:

(lldb) expr int $x = 10

the expression has no result. The ValueObject (and then SBValue) you get back from the expression signals that by putting an error indicating this in the Status object returned by SBValue::GetError(). Unfortunately, this error was not terribly helpful. The error number was a private constant that only UserExpression knew about, and the error string was unset. This meant you couldn't really trust the result of SBValue.GetError().Success() when you ran an expression.

This commit adds an eExpressionProducedNoResult constant to the ExpressionResults enum, and used that and an appropriate string in the error object. So now you can usefully tell the difference between an expression that produces no result and one that failed.

include/lldb/Expression/UserExpression.h

public:
   //------------------------------------------------------------------
   static lldb::ExpressionResults
   Evaluate(ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
            llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
            lldb::ValueObjectSP &result_valobj_sp, Status &error,
            uint32_t line_offset = 0, std::string *fixed_expression = nullptr,
            lldb::ModuleSP *jit_module_sp_ptr = nullptr);
 
-  static const Status::ValueType kNoResult =
-      0x1001; ///< ValueObject::GetError() returns this if there is no result
-              /// from the expression.
-
   const char *GetFixedText() {
     if (m_fixed_text.empty())
       return nullptr;
     return m_fixed_text.c_str();
   }
 
 protected:
   virtual lldb::ExpressionResults

include/lldb/lldb-enumerations.h

enum ExpressionResults {
   eExpressionCompleted = 0,
   eExpressionSetupError,
   eExpressionParseError,
   eExpressionDiscarded,
   eExpressionInterrupted,
   eExpressionHitBreakpoint,
   eExpressionTimedOut,
   eExpressionResultUnavailable,
-  eExpressionStoppedForDebug
+  eExpressionStoppedForDebug,
+  eExpressionProducedNoResult
 };
 
 enum SearchDepth {
     eSearchDepthInvalid = 0,
     eSearchDepthTarget,
     eSearchDepthModule,
     eSearchDepthCompUnit,
     eSearchDepthFunction,

packages/Python/lldbsuite/test/expression_command/no-result/Makefile

+LEVEL = ../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS += -std=c99
+
+include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/expression_command/no-result/TestNoResult.py

+"""
+Test that an expression that returns no result returns a sensible error.
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestExprNoResult(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    # If your test case doesn't stress debug info, the
+    # set this to true.  That way it won't be run once for
+    # each debug info format.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_no_result(self):
+        """Run an expression that has no result, check the error."""
+        self.build()
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.sample_test()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+    def sample_test(self):
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Set a breakpoint here", self.main_source_file)
+
+        frame = thread.GetFrameAtIndex(0)
+        result = frame.EvaluateExpression("int $x = 10")
+        # No result expressions are considered to fail:
+        self.assertTrue(result.GetError().Fail(), "An expression with no result is a failure.")
+        # But the reason should be eExpressionProducedNoResult
+        self.assertEqual(result.GetError().GetError(), lldb.eExpressionProducedNoResult, 
+                         "But the right kind of failure")

packages/Python/lldbsuite/test/expression_command/no-result/main.c

+#include <stdio.h>
+
+int
+main()
+{
+  int test_var = 10;
+  printf ("Set a breakpoint here: %d.\n", test_var);
+  return 0;
+}

source/Commands/CommandObjectExpression.cpp

if (result_valobj_sp) {
 
           result_valobj_sp->Dump(*output_stream, options);
 
           if (result)
             result->SetStatus(eReturnStatusSuccessFinishResult);
         }
       } else {
         if (result_valobj_sp->GetError().GetError() ==
-            UserExpression::kNoResult) {
+            lldb::eExpressionProducedNoResult) {
           if (format != eFormatVoid &&
               m_interpreter.GetDebugger().GetNotifyVoid()) {
             error_stream->PutCString("(void)\n");
           }
 
           if (result)
             result->SetStatus(eReturnStatusSuccessFinishResult);
         } else {

source/Expression/ExpressionSourceCode.cpp

if (sc.comp_unit && sc.line_entry.IsValid()) {
       DebugMacros *dm = sc.comp_unit->GetDebugMacros();
       if (dm) {
         AddMacroState state(sc.line_entry.file, sc.line_entry.line);
         AddMacros(dm, sc.comp_unit, state, debug_macros_stream);
       }
     }
 
     ConstString object_name;
-    if (Language::LanguageIsCPlusPlus(frame->GetLanguage())) {
+    if (1 /* Language::LanguageIsCPlusPlus(frame->GetLanguage())*/) {
       if (target->GetInjectLocalVariables(&exe_ctx)) {
         lldb::VariableListSP var_list_sp =
             frame->GetInScopeVariableList(false, true);
         AddLocalVariableDecls(var_list_sp, lldb_local_var_decls);
       }
     }
   }
 

source/Expression/REPL.cpp

if (code[0] == ':') {
         bool add_to_code = true;
         bool handled = false;
         if (result_valobj_sp) {
           lldb::Format format = m_format_options.GetFormat();
 
           if (result_valobj_sp->GetError().Success()) {
             handled |= PrintOneVariable(debugger, output_sp, result_valobj_sp);
           } else if (result_valobj_sp->GetError().GetError() ==
-                     UserExpression::kNoResult) {
+                     lldb::eExpressionProducedNoResult) {
             if (format != lldb::eFormatVoid && debugger.GetNotifyVoid()) {
               error_sp->PutCString("(void)\n");
               handled = true;
             }
           }
         }
 
         if (debugger.GetPrintDecls()) {

source/Expression/UserExpression.cpp

Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS |
                                                   LIBLLDB_LOG_STEP));
 
   lldb_private::ExecutionPolicy execution_policy = options.GetExecutionPolicy();
   lldb::LanguageType language = options.GetLanguage();
   const ResultType desired_type = options.DoesCoerceToId()
                                       ? UserExpression::eResultTypeId
                                       : UserExpression::eResultTypeAny;
   lldb::ExpressionResults execution_results = lldb::eExpressionSetupError;
   
+  static const char *no_result_error = "Expression completed successfully "
+                                       "but had no result";
+
   Target *target = exe_ctx.GetTargetPtr();
   if (!target) {
     if (log)
       log->Printf("== [UserExpression::Evaluate] Passed a NULL target, can't "
                   "run expressions.");
     error.SetErrorString("expression passed a null target");
     return lldb::eExpressionSetupError;
   }
if (execution_policy == eExecutionPolicyNever &&
       if (log)
         log->Printf("== [UserExpression::Evaluate] Expression may not run, but "
                     "is not constant ==");
 
       if (!diagnostic_manager.Diagnostics().size())
         error.SetExpressionError(lldb::eExpressionSetupError,
                                  "expression needed to run but couldn't");
     } else if (execution_policy == eExecutionPolicyTopLevel) {
-      error.SetError(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
+      error.SetExpressionError(lldb::eExpressionProducedNoResult,
+                               no_result_error);
       return lldb::eExpressionCompleted;
     } else {
       if (options.InvokeCancelCallback(lldb::eExpressionEvaluationExecution)) {
         error.SetExpressionError(
             lldb::eExpressionInterrupted,
             "expression interrupted by callback before execution");
         result_valobj_sp = ValueObjectConstResult::Create(
             exe_ctx.GetBestExecutionContextScope(), error);
if (execution_policy == eExecutionPolicyNever &&
             log->Printf("== [UserExpression::Evaluate] Execution completed "
                         "normally with result %s ==",
                         result_valobj_sp->GetValueAsCString());
         } else {
           if (log)
             log->Printf("== [UserExpression::Evaluate] Execution completed "
                         "normally with no result ==");
 
-          error.SetError(UserExpression::kNoResult, lldb::eErrorTypeGeneric);
+          error.SetExpressionError(lldb::eExpressionProducedNoResult,
+                                   no_result_error);
         }
       }
     }
   }
 
   if (options.InvokeCancelCallback(lldb::eExpressionEvaluationComplete)) {
     error.SetExpressionError(
         lldb::eExpressionInterrupted,

source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

if (log)
       log->Printf("%s: couldn't evaluate expression.", __FUNCTION__);
     return false;
   }
 
   // The result of the expression is invalid
   if (!expr_result->GetError().Success()) {
     Status err = expr_result->GetError();
     // Expression returned is void, so this is actually a success
-    if (err.GetError() == UserExpression::kNoResult) {
+    if (err.GetError() == lldb::eExpressionProducedNoResult) {
       if (log)
         log->Printf("%s - expression returned void.", __FUNCTION__);
 
       result = nullptr;
       return true;
     }
 
     if (log)