[lldb] add review changes

Author: da-viper

lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h

@@ -199,7 +199,7 @@ static constexpr OptionEnumValueElement g_completion_type[] = {
     {lldb::eCustomCompletion, "custom", "Custom completion."},
     {lldb::eThreadIDCompletion, "thread-id", "Completes to a thread ID."},
     {lldb::eProcessSaveCorePluginCompletion, "plugin-name",
-     "Completes to a process save-core plugin"},
+     "Completes to an ObjectFile plugin that can save core files."},
 };
 
 llvm::StringRef RegisterNameHelpTextCallback();
@@ -316,7 +316,7 @@ static constexpr CommandObject::ArgumentTableEntry g_argument_table[] = {
     { lldb::eArgTypeModule, "module", lldb::CompletionType::eModuleCompletion, {}, { nullptr, false }, "The name of a module loaded into the current target." },
     { lldb::eArgTypeCPUName, "cpu-name", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The name of a CPU." },
     { lldb::eArgTypeCPUFeatures, "cpu-features", lldb::CompletionType::eNoCompletion, {}, { nullptr, false }, "The CPU feature string." },
-    { lldb::eArgTypeSaveCorePlugin, "plugin-name", lldb::CompletionType::eProcessSaveCorePluginCompletion, {}, { nullptr, false }, nullptr },
+    { lldb::eArgTypeSaveCorePlugin, "save-core-plugin-name", lldb::CompletionType::eProcessSaveCorePluginCompletion, {}, { nullptr, false }, nullptr },
     // clang-format on
 };
 

lldb/source/Commands/CommandCompletions.cpp

@@ -738,6 +738,7 @@ void CommandCompletions::ProcessNames(CommandInterpreter &interpreter,
   for (const ProcessInstanceInfo &info : process_infos)
     request.TryCompleteCurrentArg(info.GetNameAsStringRef());
 }
+
 void CommandCompletions::ProcessSaveCorePluginNames(
     CommandInterpreter &interpreter, CompletionRequest &request,
     SearchFilter *searcher) {

lldb/source/Commands/CommandObjectProcess.cpp

@@ -1281,24 +1281,25 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
     ~CommandOptions() override = default;
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
-      if (m_opt_def.empty()) {
-        auto orig = llvm::ArrayRef(g_process_save_core_options);
-        m_opt_def.resize(orig.size());
-        llvm::copy(g_process_save_core_options, m_opt_def.data());
-        for (OptionDefinition &value : m_opt_def) {
-          llvm::StringRef opt_name = value.long_option;
-          if (opt_name != "plugin-name")
-            continue;
-
-          llvm::SmallVector<llvm::StringRef> plugin_names =
-              PluginManager::GetSaveCorePluginNames();
-          m_plugin_enums.resize(plugin_names.size());
-          for (auto [num, val] : llvm::zip(plugin_names, m_plugin_enums)) {
-            val.string_value = num.data();
-          }
-          value.enum_values = llvm::ArrayRef(m_plugin_enums);
-          break;
+      if (!m_opt_def.empty())
+        return llvm::ArrayRef(m_opt_def);
+
+      auto orig = llvm::ArrayRef(g_process_save_core_options);
+      m_opt_def.resize(orig.size());
+      llvm::copy(g_process_save_core_options, m_opt_def.data());
+      for (OptionDefinition &value : m_opt_def) {
+        llvm::StringRef opt_name = value.long_option;
+        if (opt_name != "plugin-name")
+          continue;
+
+        llvm::SmallVector<llvm::StringRef> plugin_names =
+            PluginManager::GetSaveCorePluginNames();
+        m_plugin_enums.resize(plugin_names.size());
+        for (auto [num, val] : llvm::zip(plugin_names, m_plugin_enums)) {
+          val.string_value = num.data();
         }
+        value.enum_values = llvm::ArrayRef(m_plugin_enums);
+        break;
       }
       return llvm::ArrayRef(m_opt_def);
     }

lldb/source/Commands/Options.td

@@ -797,7 +797,7 @@ let Command = "process save_core" in {
     EnumArg<"SaveCoreStyle">, Desc<"Request a specific style "
     "of corefile to be saved.">;
   def process_save_core_plugin_name : Option<"plugin-name", "p">,
-    Arg<"Plugin">, Completion<"ProcessSaveCorePlugin">,
+    OptionalArg<"Plugin">, Completion<"ProcessSaveCorePlugin">,
     Desc<"Specify a plugin name to create the core file. "
     "This allows core files to be saved in different formats.">;
 }

lldb/source/Core/PluginManager.cpp

@@ -792,18 +792,19 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
     }
   }
 
-  // report for one plugin if a name is specified.
-  if (error.Success() && !plugin_name.empty())
-    error = Status::FromErrorStringWithFormatv(
-        "\"{}\" plugin is not able to save a core for this process.",
+  // Check to see if any of the object file plugins tried and failed to save.
+  // if any failed resturn the error message.
+  if (error.Fail())
+    return error;
+
+  // Report only for the plugin that was specified.
+  if (!plugin_name.empty())
+    return Status::FromErrorStringWithFormatv(
+        "The \"{}\" plugin is not able to save a core for this process.",
         plugin_name);
 
-  // Check to see if any of the object file plugins tried and failed to save.
-  // If none ran, set the error message.
-  if (error.Success())
-    error = Status::FromErrorString(
-        "no ObjectFile plugins were able to save a core for this process");
-  return error;
+  return Status::FromErrorString(
+      "no ObjectFile plugins were able to save a core for this process");
 }
 
 llvm::SmallVector<llvm::StringRef> PluginManager::GetSaveCorePluginNames() {

lldb/source/Symbol/SaveCoreOptions.cpp

@@ -21,15 +21,15 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
     return error;
   }
 
-  if (!PluginManager::IsRegisteredObjectFilePluginName(name)) {
+  llvm::SmallVector<llvm::StringRef> plugin_names =
+      PluginManager::GetSaveCorePluginNames();
+  if (llvm::find(plugin_names, name) == plugin_names.end()) {
     StreamString stream;
     stream.Printf("plugin name '%s' is not a valid ObjectFile plugin name.",
                   name);
 
-    llvm::SmallVector<llvm::StringRef> plugin_names =
-        PluginManager::GetSaveCorePluginNames();
     if (!plugin_names.empty()) {
-      stream.PutCString(" Valid values are: ");
+      stream.PutCString(" Valid names are: ");
       std::string plugin_names_str = llvm::join(plugin_names, ", ");
       stream.PutCString(plugin_names_str);
       stream.PutChar('.');