[lldb] Remove more redundant SetStatus(eReturnStatusFailed)

[DavidSpickett]

Summary

Mostly by converting uses of GetErrorStream to AppendError,
so that the call to SetStatus is implicit.

Some remain where it isn't certain that you'll have a message
to set, or you want the output to be on stdout.

One place in CommandObjectWatchpoint previously didn't set
the status to failed at all. However it's pretty obvious
that it should do so.

[teemperor]

lldb/source/Commands/CommandObjectReproducer.cpp

141

You can drop the c_str()

[DavidSpickett]

Remove c_str() in Reproducer.

lldb/source/Commands/CommandObjectPlatform.cpp

else if (request.GetCursorIndex() == 1)
       CommandCompletions::InvokeCommonCompletionCallbacks(
           GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
           request, nullptr);
   }
 
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     // If the number of arguments is incorrect, issue an error message.
     if (args.GetArgumentCount() != 2) {
-      result.GetErrorStream().Printf("error: required arguments missing; "
-                                     "specify both the source and destination "
-                                     "file paths\n");
-      result.SetStatus(eReturnStatusFailed);
+      result.AppendError("required arguments missing; specify both the "
+                         "source and destination file paths");
       return false;
     }
 
     PlatformSP platform_sp(
         GetDebugger().GetPlatformList().GetSelectedPlatform());
     if (platform_sp) {
       const char *remote_file_path = args.GetArgumentAtIndex(0);
       const char *local_file_path = args.GetArgumentAtIndex(1);
HandleArgumentCompletion(CompletionRequest &request,
     CommandCompletions::InvokeCommonCompletionCallbacks(
         GetCommandInterpreter(), CommandCompletions::eRemoteDiskFileCompletion,
         request, nullptr);
   }
 
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     // If the number of arguments is incorrect, issue an error message.
     if (args.GetArgumentCount() != 1) {
-      result.GetErrorStream().Printf("error: required argument missing; "
-                                     "specify the source file path as the only "
-                                     "argument\n");
-      result.SetStatus(eReturnStatusFailed);
+      result.AppendError("required argument missing; specify the source file "
+                         "path as the only argument");
       return false;
     }
 
     PlatformSP platform_sp(
         GetDebugger().GetPlatformList().GetSelectedPlatform());
     if (platform_sp) {
       std::string remote_file_path(args.GetArgumentAtIndex(0));
       user_id_t size = platform_sp->GetFileSize(FileSpec(remote_file_path));

lldb/source/Commands/CommandObjectReproducer.cpp

llvm::Expected<T> static ReadFromYAML(StringRef filename) {
   if (auto err = yin.error()) {
     return errorCodeToError(err);
   }
 
   return t;
 }
 
 static void SetError(CommandReturnObject &result, Error err) {
-  result.GetErrorStream().Printf("error: %s\n",
-                                 toString(std::move(err)).c_str());
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError(toString(std::move(err)));
 }
 
 /// Create a loader from the given path if specified. Otherwise use the current
 /// loader used for replay.
 static Loader *
 GetLoaderFromPathOrCurrent(llvm::Optional<Loader> &loader_storage,
                            CommandReturnObject &result,
                            FileSpec reproducer_path) {

lldb/source/Commands/CommandObjectTarget.cpp

if (argc == 1 || core_file || remote_file) {
       llvm::StringRef arch_cstr = m_arch_option.GetArchitectureName();
       Status error(debugger.GetTargetList().CreateTarget(
           debugger, file_path, arch_cstr,
           m_add_dependents.m_load_dependent_files, &m_platform_options,
           target_sp));
 
       if (!target_sp) {
         result.AppendError(error.AsCString());
-        result.SetStatus(eReturnStatusFailed);
         return false;
       }
 
       auto on_error = llvm::make_scope_exit(
           [&target_list = debugger.GetTargetList(), &target_sp]() {
             target_list.DeleteTarget(target_sp);
           });
 
       // Only get the platform after we create the target because we might
       // have switched platforms depending on what the arguments were to
       // CreateTarget() we can't rely on the selected platform.
 
       PlatformSP platform_sp = target_sp->GetPlatform();
 
       if (remote_file) {
         if (platform_sp) {
           // I have a remote file.. two possible cases
           if (file_spec && FileSystem::Instance().Exists(file_spec)) {
             // if the remote file does not exist, push it there
             if (!platform_sp->GetFileExists(remote_file)) {
               Status err = platform_sp->PutFile(file_spec, remote_file);
               if (err.Fail()) {
                 result.AppendError(err.AsCString());
-                result.SetStatus(eReturnStatusFailed);
                 return false;
               }
             }
           } else {
             // there is no local file and we need one
             // in order to make the remote ---> local transfer we need a
             // platform
             // TODO: if the user has passed in a --platform argument, use it
             // to fetch the right platform
             if (file_path) {
               // copy the remote file to the local file
               Status err = platform_sp->GetFile(remote_file, file_spec);
               if (err.Fail()) {
                 result.AppendError(err.AsCString());
-                result.SetStatus(eReturnStatusFailed);
                 return false;
               }
             } else {
               // make up a local file
               result.AppendError("remote --> local transfer without local "
                                  "path is not implemented yet");
               return false;
             }
if (argc > 0) {
         } else {
           Status error(Variable::GetValuesForVariableExpressionPath(
               arg.c_str(), m_exe_ctx.GetBestExecutionContextScope(),
               GetVariableCallback, target, variable_list, valobj_list));
           matches = variable_list.GetSize();
         }
 
         if (matches == 0) {
-          result.GetErrorStream().Printf(
-              "error: can't find global variable '%s'\n", arg.c_str());
-          result.SetStatus(eReturnStatusFailed);
+          result.AppendErrorWithFormat("can't find global variable '%s'",
+                                       arg.c_str());
           return false;
         } else {
           for (uint32_t global_idx = 0; global_idx < matches; ++global_idx) {
             VariableSP var_sp(variable_list.GetVariableAtIndex(global_idx));
             if (var_sp) {
               ValueObjectSP valobj_sp(
                   valobj_list.GetValueObjectAtIndex(global_idx));
               if (!valobj_sp)
if (argc == 0) {
               module_spec, true /* notify */, &error));
           if (!module_sp) {
             const char *error_cstr = error.AsCString();
             if (error_cstr)
               result.AppendError(error_cstr);
             else
               result.AppendErrorWithFormat("unsupported module: %s",
                                            entry.c_str());
-            result.SetStatus(eReturnStatusFailed);
             return false;
           } else {
             flush = true;
           }
           result.SetStatus(eReturnStatusSuccessFinishResult);
         } else {
           std::string resolved_path = file_spec.GetPath();
           if (resolved_path != entry.ref()) {

lldb/source/Commands/CommandObjectVersion.cpp

 CommandObjectVersion::~CommandObjectVersion() {}
 
 bool CommandObjectVersion::DoExecute(Args &args, CommandReturnObject &result) {
   if (args.GetArgumentCount() == 0) {
     result.AppendMessageWithFormat("%s\n", lldb_private::GetVersion());
     result.SetStatus(eReturnStatusSuccessFinishResult);
   } else {
     result.AppendError("the version command takes no arguments.");
-    result.SetStatus(eReturnStatusFailed);
   }
   return true;
 }

lldb/source/Commands/CommandObjectWatchpoint.cpp

if (valobj_sp) {
         size = m_option_watchpoint.watch_size == 0
                    ? valobj_sp->GetByteSize().getValueOr(0)
                    : m_option_watchpoint.watch_size;
       }
       compiler_type = valobj_sp->GetCompilerType();
     } else {
       const char *error_cstr = error.AsCString(nullptr);
       if (error_cstr)
-        result.GetErrorStream().Printf("error: %s\n", error_cstr);
+        result.AppendError(error_cstr);
       else
-        result.GetErrorStream().Printf("error: unable to find any variable "
-                                       "expression path that matches '%s'\n",
-                                       command.GetArgumentAtIndex(0));
+        result.AppendErrorWithFormat("unable to find any variable "
+                                     "expression path that matches '%s'",
       return false;
     }
 
     // Now it's time to create the watchpoint.
     uint32_t watch_type = m_option_watchpoint.watch_type;
 
     error.Clear();
     Watchpoint *wp =
bool DoExecute(llvm::StringRef raw_command,
     if (args.HasArgs())
       if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group,
                                  exe_ctx))
         return false;
 
     // If no argument is present, issue an error message.  There's no way to
     // set a watchpoint.
     if (raw_command.trim().empty()) {
-      result.GetErrorStream().Printf("error: required argument missing; "
-                                     "specify an expression to evaluate into "
-                                     "the address to watch for\n");
-      result.SetStatus(eReturnStatusFailed);
+      result.AppendError("required argument missing; specify an expression "
+                         "to evaluate into the address to watch for");
       return false;
     }
 
     // If no '-w' is specified, default to '-w write'.
     if (!m_option_watchpoint.watch_type_specified) {
       m_option_watchpoint.watch_type = OptionGroupWatchpoint::eWatchWrite;
     }
 
bool DoExecute(llvm::StringRef raw_command,
     options.SetUnwindOnError(true);
     options.SetKeepInMemory(false);
     options.SetTryAllThreads(true);
     options.SetTimeout(llvm::None);
 
     ExpressionResults expr_result =
         target->EvaluateExpression(expr, frame, valobj_sp, options);
     if (expr_result != eExpressionCompleted) {
-      result.GetErrorStream().Printf(
-          "error: expression evaluation of address to watch failed\n");
-      result.GetErrorStream() << "expression evaluated: \n" << expr << "\n";
+      result.AppendError("expression evaluation of address to watch failed");
+      result.AppendErrorWithFormat("expression evaluated: \n%s", expr.data());
       if (valobj_sp && !valobj_sp->GetError().Success())
-        result.GetErrorStream() << valobj_sp->GetError().AsCString() << "\n";
-      result.SetStatus(eReturnStatusFailed);
+        result.AppendError(valobj_sp->GetError().AsCString());
       return false;
     }
 
     // Get the address to watch.
     bool success = false;
     addr = valobj_sp->GetValueAsUnsigned(0, &success);
     if (!success) {
-      result.GetErrorStream().Printf(
-          "error: expression did not evaluate to an address\n");
-      result.SetStatus(eReturnStatusFailed);
+      result.AppendError("expression did not evaluate to an address");
       return false;
     }
 
     if (m_option_watchpoint.watch_size != 0)
       size = m_option_watchpoint.watch_size;
     else
       size = target->GetArchitecture().GetAddressByteSize();
 

lldb/source/Interpreter/CommandObject.cpp

bool CommandObject::CheckRequirements(CommandReturnObject &result) {
 
   if (GetFlags().AnySet(eCommandProcessMustBeLaunched |
                         eCommandProcessMustBePaused)) {
     Process *process = m_interpreter.GetExecutionContext().GetProcessPtr();
     if (process == nullptr) {
       // A process that is not running is considered paused.
       if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
         result.AppendError("Process must exist.");
-        result.SetStatus(eReturnStatusFailed);
         return false;
       }
     } else {
       StateType state = process->GetState();
       switch (state) {
       case eStateInvalid:
       case eStateSuspended:
       case eStateCrashed:
       case eStateStopped:
         break;
 
       case eStateConnected:
       case eStateAttaching:
       case eStateLaunching:
       case eStateDetached:
       case eStateExited:
       case eStateUnloaded:
         if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
           result.AppendError("Process must be launched.");
-          result.SetStatus(eReturnStatusFailed);
           return false;
         }
         break;
 
       case eStateRunning:
       case eStateStepping:
         if (GetFlags().Test(eCommandProcessMustBePaused)) {
           result.AppendError("Process is running.  Use 'process interrupt' to "
                              "pause execution.");
-          result.SetStatus(eReturnStatusFailed);
           return false;
         }
       }
     }
   }
 
   if (GetFlags().Test(eCommandProcessMustBeTraced)) {
     Target *target = m_exe_ctx.GetTargetPtr();
bool CommandObject::ParseOptionsAndNotify(Args &args,
                                           OptionGroupOptions &group_options,
                                           ExecutionContext &exe_ctx) {
   if (!ParseOptions(args, result))
     return false;
 
   Status error(group_options.NotifyOptionParsingFinished(&exe_ctx));
   if (error.Fail()) {
     result.AppendError(error.AsCString());
-    result.SetStatus(eReturnStatusFailed);
     return false;
   }
   return true;
 }
 
 int CommandObject::GetNumArgumentEntries() { return m_arguments.size(); }
 
 CommandObject::CommandArgumentEntry *