[DWIMPrint] Move the setting of the result status into dump_val_object

[adrian-prantl]

Previously the result would get overwritten by a success on all code paths.

This is another NFC change for TypeSystemClang, because an object description cannot actually fail there. It will have different behavior in the Swift plugin.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

This is another NFC change for TypeSystemClang, because an object description cannot actually fail there. It will have different behavior in the Swift plugin.

---

Full diff: https://github.com/llvm/llvm-project/pull/96232.diff

1 Files Affected:

• (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+31-27)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index c1549ca6933fc..b7cd955e00203 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -141,10 +141,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
       maybe_add_hint(output);
       result.GetOutputStream() << output;
     } else {
-      if (llvm::Error error =
-              valobj.Dump(result.GetOutputStream(), dump_options))
+      llvm::Error error =
+        valobj.Dump(result.GetOutputStream(), dump_options);
+      if (error) {
         result.AppendError(toString(std::move(error)));
+        return;
+      }
     }
+    result.SetStatus(eReturnStatusSuccessFinishResult);
   };
 
   // First, try `expr` as the name of a frame variable.
@@ -165,7 +169,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
       }
 
       dump_val_object(*valobj_sp);
-      result.SetStatus(eReturnStatusSuccessFinishResult);
       return;
     }
   }
@@ -176,7 +179,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
       if (auto var_sp = state->GetVariable(expr))
         if (auto valobj_sp = var_sp->GetValueObject()) {
           dump_val_object(*valobj_sp);
-          result.SetStatus(eReturnStatusSuccessFinishResult);
           return;
         }
 
@@ -197,34 +199,36 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
       error_stream << "    " << fixed_expression << "\n";
     }
 
-    if (expr_result == eExpressionCompleted) {
-      if (verbosity != eDWIMPrintVerbosityNone) {
-        StringRef flags;
-        if (args.HasArgs())
-          flags = args.GetArgStringWithDelimiter();
-        result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
-                                        expr);
-      }
-
-      if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
-        dump_val_object(*valobj_sp);
-
-      if (suppress_result)
-        if (auto result_var_sp =
-                target.GetPersistentVariable(valobj_sp->GetName())) {
-          auto language = valobj_sp->GetPreferredDisplayLanguage();
-          if (auto *persistent_state =
-                  target.GetPersistentExpressionStateForLanguage(language))
-            persistent_state->RemovePersistentVariable(result_var_sp);
-        }
-
-      result.SetStatus(eReturnStatusSuccessFinishResult);
-    } else {
+    // If the expression failed, return an error.
+    if (expr_result != eExpressionCompleted) {
       if (valobj_sp)
         result.SetError(valobj_sp->GetError());
       else
         result.AppendErrorWithFormatv(
             "unknown error evaluating expression `{0}`", expr);
+      return;
+    }
+
+    if (verbosity != eDWIMPrintVerbosityNone) {
+      StringRef flags;
+      if (args.HasArgs())
+        flags = args.GetArgStringWithDelimiter();
+      result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
+                                      expr);
     }
+
+    if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
+      dump_val_object(*valobj_sp);
+    else
+      result.SetStatus(eReturnStatusSuccessFinishResult);
+
+    if (suppress_result)
+      if (auto result_var_sp =
+              target.GetPersistentVariable(valobj_sp->GetName())) {
+        auto language = valobj_sp->GetPreferredDisplayLanguage();
+        if (auto *persistent_state =
+                target.GetPersistentExpressionStateForLanguage(language))
+          persistent_state->RemovePersistentVariable(result_var_sp);
+      }
   }
 }

[kastiglione]

thanks