[lldb/Commands] Fix, rename and document column number argument to breakpoint set.

[JDevlieghere]

Summary

We were incorrectly parsing the -C argument to breakpoint set as the column breakpoint, even though according to the help this should be the breakpoint command. This fixes that by renaming the option to -u, adding it to help, and adding a test case.

[jingham]

LGTM. This is my bad. I put in the parsing for -C in CommandObjectBreakpointSet in an (apparently futile) attempt to remind myself to reserve -C for column number...

lldb/include/lldb/lldb-enumerations.h

enum CommandArgumentType {
   eArgTypeWidth,
   eArgTypeNone,
   eArgTypePlatform,
   eArgTypeWatchpointID,
   eArgTypeWatchpointIDRange,
   eArgTypeWatchType,
   eArgRawInput,
   eArgTypeCommand,
+  eArgTypeColumnNum,
   eArgTypeLastArg // Always keep this entry as the last entry in this
                   // enumeration!!
 };
 
 /// Symbol types.
 // Symbol holds the SymbolType in a 6-bit field (m_type), so if you get over 63
 // entries you will have to resize that field.
 enum SymbolType {

lldb/source/Commands/CommandObjectBreakpoint.cpp

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         m_all_files = true;
         break;
 
       case 'b':
         m_func_names.push_back(option_arg);
         m_func_name_type_mask |= eFunctionNameTypeBase;
         break;
 
-      case 'C':
+      case 'u':
         if (option_arg.getAsInteger(0, m_column))
           error.SetErrorStringWithFormat("invalid column number: %s",
                                          option_arg.str().c_str());
         break;
 
       case 'E': {
         LanguageType language = Language::GetLanguageTypeFromString(option_arg);
 

lldb/source/Commands/Options.td

def breakpoint_set_file : Option<"file", "f">, Arg<"Filename">,
     Desc<"Specifies the source file in which to set this breakpoint.  Note, by "
     "default lldb only looks for files that are #included if they use the "
     "standard include file extensions.  To set breakpoints on .c/.cpp/.m/.mm "
     "files that are #included, set target.inline-breakpoint-strategy to "
     "\"always\".">;
   def breakpoint_set_line : Option<"line", "l">, Group<1>, Arg<"LineNum">,
     Required,
     Desc<"Specifies the line number on which to set this breakpoint.">;
+  def breakpoint_set_column : Option<"column", "u">, Group<1>, Arg<"ColumnNum">,
+    Desc<"Specifies the column number on which to set this breakpoint.">;
   def breakpoint_set_address : Option<"address", "a">, Group<2>,
     Arg<"AddressOrExpression">, Required,
     Desc<"Set the breakpoint at the specified address.  If the address maps "
     "uniquely toa particular binary, then the address will be converted to "
     "a \"file\"address, so that the breakpoint will track that binary+offset "
     "no matter where the binary eventually loads.  Alternately, if you also "
     "specify the module - with the -s option - then the address will be "
     "treated as a file address in that module, and resolved accordingly.  "

lldb/source/Interpreter/CommandObject.cpp

CommandObject::ArgumentTableEntry CommandObject::g_arguments_data[] = {
     { eArgTypeValue, "value", CommandCompletions::eNoCompletion, { nullptr, false }, "A value could be anything, depending on where and how it is used." },
     { eArgTypeWidth, "width", CommandCompletions::eNoCompletion, { nullptr, false }, "Help text goes here." },
     { eArgTypeNone, "none", CommandCompletions::eNoCompletion, { nullptr, false }, "No help available for this." },
     { eArgTypePlatform, "platform-name", CommandCompletions::ePlatformPluginCompletion, { nullptr, false }, "The name of an installed platform plug-in . Type 'platform list' to see a complete list of installed platforms." },
     { eArgTypeWatchpointID, "watchpt-id", CommandCompletions::eNoCompletion, { nullptr, false }, "Watchpoint IDs are positive integers." },
     { eArgTypeWatchpointIDRange, "watchpt-id-list", CommandCompletions::eNoCompletion, { nullptr, false }, "For example, '1-3' or '1 to 3'." },
     { eArgTypeWatchType, "watch-type", CommandCompletions::eNoCompletion, { nullptr, false }, "Specify the type for a watchpoint." },
     { eArgRawInput, "raw-input", CommandCompletions::eNoCompletion, { nullptr, false }, "Free-form text passed to a command without prior interpretation, allowing spaces without requiring quotes.  To pass arguments and free form text put two dashes ' -- ' between the last argument and any raw input." },
-    { eArgTypeCommand, "command", CommandCompletions::eNoCompletion, { nullptr, false }, "An LLDB Command line command." }
+    { eArgTypeCommand, "command", CommandCompletions::eNoCompletion, { nullptr, false }, "An LLDB Command line command." },
+    { eArgTypeColumnNum, "column", CommandCompletions::eNoCompletion, { nullptr, false }, "Column number in a source file." }
     // clang-format on
 };
 
 const CommandObject::ArgumentTableEntry *CommandObject::GetArgumentTable() {
   // If this assertion fires, then the table above is out of date with the
   // CommandArgumentType enumeration
   static_assert((sizeof(CommandObject::g_arguments_data) /
                  sizeof(CommandObject::ArgumentTableEntry)) == eArgTypeLastArg,
                 "");
   return CommandObject::g_arguments_data;
 }

lldb/test/Shell/Commands/command-breakpoint-col.test

+# RUN: %clang_host -g -O0 %S/Inputs/main.c -o %t.out
+# RUN: %lldb -b -o 'help breakpoint set' -o 'breakpoint set -f main.c -l 2 -u 21' %t.out | FileCheck %s
+# CHECK: -u <column> ( --column <column> )
+# CHECK: Specifies the column number on which to set this breakpoint.
+# CHECK: at main.c:2:21