[Lldb-commits] [lldb] Add a `breakpoint add` command to fix the option-madness that is `breakpoint set` (PR #156067)

[Jonas Devlieghere via lldb-commits]

================
@@ -200,41 +200,1341 @@ class lldb_private::BreakpointOptionGroup : public OptionGroup {
   BreakpointOptions m_bp_opts;
 };
 
+// This is the Breakpoint Names option group - used to add Names to breakpoints
+// while making them.  Not to be confused with the "Breakpoint Name" option
+// group which is the common options of various "breakpoint name" commands.
+#define LLDB_OPTIONS_breakpoint_names
+#include "CommandOptions.inc"
+
+class BreakpointNamesOptionGroup : public OptionGroup {
+public:
+  BreakpointNamesOptionGroup() = default;
+
+  ~BreakpointNamesOptionGroup() override = default;
+
+  llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+    return g_breakpoint_names_options;
+  }
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+                        ExecutionContext *execution_context) override {
+    Status error;
+    const int short_option = GetDefinitions()[option_idx].short_option;
+    const char *long_option = GetDefinitions()[option_idx].long_option;
+
+    switch (short_option) {
+    case 'N':
+      if (BreakpointID::StringIsBreakpointName(option_value, error))
+        m_breakpoint_names.push_back(std::string(option_value));
+      else
+        error = Status::FromError(
+            CreateOptionParsingError(option_value, short_option, long_option,
+                                     "Invalid breakpoint name"));
+      break;
+    }
+    return error;
+  }
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+    m_breakpoint_names.clear();
+  }
+
+  const std::vector<std::string> &GetBreakpointNames() {
+    return m_breakpoint_names;
+  }
+
+protected:
+  std::vector<std::string> m_breakpoint_names;
+};
+
 #define LLDB_OPTIONS_breakpoint_dummy
 #include "CommandOptions.inc"
 
-class BreakpointDummyOptionGroup : public OptionGroup {
+class BreakpointDummyOptionGroup : public OptionGroup {
+public:
+  BreakpointDummyOptionGroup() = default;
+
+  ~BreakpointDummyOptionGroup() override = default;
+
+  llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+    return llvm::ArrayRef(g_breakpoint_dummy_options);
+  }
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+                        ExecutionContext *execution_context) override {
+    Status error;
+    const int short_option =
+        g_breakpoint_dummy_options[option_idx].short_option;
+
+    switch (short_option) {
+    case 'D':
+      m_use_dummy = true;
+      break;
+    default:
+      llvm_unreachable("Unimplemented option");
+    }
+
+    return error;
+  }
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override {
+    m_use_dummy = false;
+  }
+
+  bool m_use_dummy;
+};
+
+#pragma mark AddAddress::CommandOptions
+#define LLDB_OPTIONS_breakpoint_add_address
+#include "CommandOptions.inc"
+
+#pragma mark Add Address
+
+static bool CopyOverBreakpointOptions(BreakpointSP bp_sp,
+                                      BreakpointOptionGroup &bp_opts,
+                                      const std::vector<std::string> &bp_names,
+                                      CommandReturnObject &result) {
+  assert(bp_sp && "CopyOverBreakpointOptions called with no breakpoint");
+
+  bp_sp->GetOptions().CopyOverSetOptions(bp_opts.GetBreakpointOptions());
+  Target &target = bp_sp->GetTarget();
+  if (!bp_names.empty()) {
+    Status name_error;
+    for (auto name : bp_names) {
+      target.AddNameToBreakpoint(bp_sp, name.c_str(), name_error);
+      if (name_error.Fail()) {
+        result.AppendErrorWithFormat("Invalid breakpoint name: %s",
----------------
JDevlieghere wrote:

Errors should start with lowercase letters (https://llvm.org/docs/CodingStandards.html#error-and-warning-messages)

https://github.com/llvm/llvm-project/pull/156067