Refine the 'watchpoint set' command to now require either the '-v' option (for watching of a variable) or
the '-e' option (for watching of an address) to be present.

Update some existing test cases with the required option and add some more test cases.

Since the '-v' option takes <variable-name> and the '-e' option takes <expr> as the command arg,
the existing infrastructure for generating the option usage can produce confusing help message,
like:

  watchpoint set -e [-w <watch-type>] [-x <byte-size>] <variable-name | expr>
  watchpoint set -v [-w <watch-type>] [-x <byte-size>] <variable-name | expr>

The solution adopted is to provide an extra member field to the struct CommandArgumentData called
(uint32_t)arg_opt_set_association, whose purpose is to link this particular argument data with some
option set(s).  Also modify the signature of CommandObject::GetFormattedCommandArguments() to:

  GetFormattedCommandArguments (Stream &str, uint32_t opt_set_mask = LLDB_OPT_SET_ALL)

it now takes an additional opt_set_mask which can be used to generate a filtered formatted command
args for help message.

Options::GenerateOptionUsage() impl is modified to call the GetFormattedCommandArguments() appropriately.
So that the help message now looks like:

  watchpoint set -e [-w <watch-type>] [-x <byte-size>] <expr>
  watchpoint set -v [-w <watch-type>] [-x <byte-size>] <variable-name>

rdar://problem/10703256


git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@150032 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/source/Commands/CommandObjectWatchpoint.cpp b/source/Commands/CommandObjectWatchpoint.cpp
index f72a900..bb715d8 100644
--- a/source/Commands/CommandObjectWatchpoint.cpp
+++ b/source/Commands/CommandObjectWatchpoint.cpp
@@ -853,6 +853,70 @@
 }
 
 //-------------------------------------------------------------------------
+// CommandObjectWatchpointSet::CommandOptions
+//-------------------------------------------------------------------------
+#pragma mark Set::CommandOptions
+
+CommandObjectWatchpointSet::CommandOptions::CommandOptions() :
+    OptionGroup()
+{
+}
+
+CommandObjectWatchpointSet::CommandOptions::~CommandOptions ()
+{
+}
+
+OptionDefinition
+CommandObjectWatchpointSet::CommandOptions::g_option_table[] =
+{
+{ LLDB_OPT_SET_1, true, "expression", 'e', no_argument, NULL, NULL, eArgTypeNone, "Watch an address with an expression specified at the end."},
+{ LLDB_OPT_SET_2, true, "variable",   'v', no_argument, NULL, NULL, eArgTypeNone, "Watch a variable name specified at the end."}
+};
+
+uint32_t
+CommandObjectWatchpointSet::CommandOptions::GetNumDefinitions ()
+{
+    return sizeof(g_option_table)/sizeof(OptionDefinition);
+}
+
+const OptionDefinition*
+CommandObjectWatchpointSet::CommandOptions::GetDefinitions ()
+{
+    return g_option_table;
+}
+
+Error
+CommandObjectWatchpointSet::CommandOptions::SetOptionValue (CommandInterpreter &interpreter,
+                                                            uint32_t option_idx,
+                                                            const char *option_arg)
+{
+    Error error;
+    char short_option = (char) g_option_table[option_idx].short_option;
+
+    switch (short_option)
+    {
+        case 'e':
+            m_do_expression = true;
+            break;
+        case 'v':
+            m_do_variable = true;
+            break;
+        default:
+            error.SetErrorStringWithFormat ("unrecognized option '%c'", short_option);
+            break;
+    }
+
+    return error;
+}
+
+void
+CommandObjectWatchpointSet::CommandOptions::OptionParsingStarting (CommandInterpreter &interpreter)
+{
+    m_do_expression = false;
+    m_do_variable = false;
+}
+
+//-------------------------------------------------------------------------
 // CommandObjectWatchpointSet
 //-------------------------------------------------------------------------
 #pragma mark Set
@@ -861,45 +925,51 @@
     CommandObject (interpreter,
                    "watchpoint set",
                    "Set a watchpoint. "
-                   "You can choose to watch a variable in scope with just the '-w' option. "
-                   "If you use the '-x' option to specify the byte size, it is implied "
-                   "that the remaining string is evaluated as an expression with the result "
-                   "interpreted as an address to watch for, i.e., the pointee is watched. "
+                   "You can choose to watch a variable in scope with the '-v' option "
+                   "or to watch an address with the '-e' option by supplying an expression. "
+                   "Use the '-w' option to specify the type of watchpoint and "
+                   "the '-x' option to specify the byte size to watch for. "
                    "If no '-w' option is specified, it defaults to read_write. "
                    "Note that hardware resources for watching are often limited.",
                    NULL,
                    eFlagProcessMustBeLaunched | eFlagProcessMustBePaused),
     m_option_group (interpreter),
-    m_option_watchpoint()
+    m_option_watchpoint (),
+    m_command_options ()
 {
     SetHelpLong(
 "Examples: \n\
 \n\
-    watchpoint set -w read_wriate my_global_var \n\
-    # Watch my_global_var for read/write access.\n\
+    watchpoint set -w read_wriate -v my_global_var \n\
+    # Watch my_global_var for read/write access, with the region to watch corresponding to the byte size of the data type.\n\
 \n\
-    watchpoint set -w write -x 1 foo + 32\n\
-    # Watch write access for the 1-byte region pointed to by the address 'foo + 32'.\n");
+    watchpoint set -w write -x 1 -e foo + 32\n\
+    # Watch write access for the 1-byte region pointed to by the address 'foo + 32'.\n\
+    # If no '-x' option is specified, byte size defaults to 4.\n");
 
     CommandArgumentEntry arg;
     CommandArgumentData var_name_arg, expression_arg;
         
     // Define the first variant of this arg.
-    var_name_arg.arg_type = eArgTypeVarName;
-    var_name_arg.arg_repetition = eArgRepeatPlain;
-
-    // Define the second variant of this arg.
     expression_arg.arg_type = eArgTypeExpression;
     expression_arg.arg_repetition = eArgRepeatPlain;
+    expression_arg.arg_opt_set_association = LLDB_OPT_SET_1;
         
+    // Define the second variant of this arg.
+    var_name_arg.arg_type = eArgTypeVarName;
+    var_name_arg.arg_repetition = eArgRepeatPlain;
+    var_name_arg.arg_opt_set_association = LLDB_OPT_SET_2;
+
     // Push the two variants into the argument entry.
-    arg.push_back (var_name_arg);
     arg.push_back (expression_arg);
+    arg.push_back (var_name_arg);
         
-    // Push the data for the first argument into the m_arguments vector.
+    // Push the data for the only argument into the m_arguments vector.
     m_arguments.push_back (arg);
-        
-    m_option_group.Append (&m_option_watchpoint, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
+
+    // Absorb the '-w' and '-x' options.
+    m_option_group.Append (&m_option_watchpoint, LLDB_OPT_SET_1, LLDB_OPT_SET_1|LLDB_OPT_SET_2);
+    m_option_group.Append (&m_command_options);
     m_option_group.Finalize();
 }
 
@@ -930,35 +1000,25 @@
         return false;
     }
 
-    // Be careful about the stack frame, if any summary formatter runs code, it might clear the StackFrameList
-    // for the thread.  So hold onto a shared pointer to the frame so it stays alive.
-    bool get_file_globals = true;
-    VariableList *variable_list = frame->GetVariableList (get_file_globals);
+    // If no argument is present, issue an error message.  There's no way to set a watchpoint.
+    if (command.GetArgumentCount() <= 0)
+    {
+        result.GetErrorStream().Printf("error: required argument missing; specify your program variable ('-v') or an address ('-e') to watch for\n");
+        result.SetStatus(eReturnStatusFailed);
+        return false;
+    }
 
-    bool watch_address = (m_option_watchpoint.watch_size > 0);
-        
+    // It's either '-e' to watch an address with expression' or '-v' to watch a variable.
+    bool watch_address = m_command_options.m_do_expression;
+
     // If no '-w' is specified, default to '-w read_write'.
     if (!m_option_watchpoint.watch_variable)
     {
         m_option_watchpoint.watch_variable = true;
         m_option_watchpoint.watch_type = OptionGroupWatchpoint::eWatchReadWrite;
     }
-    // It's possible to specify an address to watch for with the '-x' option.
-    if (!variable_list && !watch_address)
-    {
-        result.GetErrorStream().Printf("error: no variables found, did you forget to use '-x' option to watch an address?\n");
-        result.SetStatus(eReturnStatusFailed);
-        return false;
-    }
-    // If thre's no argument, it is an error.
-    if (command.GetArgumentCount() <= 0)
-    {
-        result.GetErrorStream().Printf("error: specify your target variable (no '-x') or expression (with '-x') to watch for\n");
-        result.SetStatus(eReturnStatusFailed);
-        return false;
-    }        
 
-    // We passed the sanity check for the options.
+    // We passed the sanity check for the command.
     // Proceed to set the watchpoint now.
     lldb::addr_t addr = 0;
     size_t size = 0;
@@ -968,6 +1028,7 @@
     Stream &output_stream = result.GetOutputStream();
 
     if (watch_address) {
+        // Use expression evaluation to arrive at the address to watch.
         std::string expr_str;
         command.GetQuotedCommandString(expr_str);
         const bool coerce_to_id = true;
@@ -983,6 +1044,7 @@
                                                                    valobj_sp);
         if (expr_result != eExecutionCompleted) {
             result.GetErrorStream().Printf("error: expression evaluation of address to watch failed\n");
+            result.GetErrorStream().Printf("expression evaluated: %s\n", expr_str.c_str());
             result.SetStatus(eReturnStatusFailed);
         }
 
@@ -993,11 +1055,12 @@
             result.SetStatus(eReturnStatusFailed);
             return false;
         }
-        size = m_option_watchpoint.watch_size;
+        size = m_option_watchpoint.watch_size == 0 ? 4 /* Could use a better default size? */
+                                                   : m_option_watchpoint.watch_size;
     } else {
         // A simple watch variable gesture allows only one argument.
-        if (m_option_watchpoint.watch_size == 0 && command.GetArgumentCount() != 1) {
-            result.GetErrorStream().Printf("error: specify exactly one variable with the '-w' option, i.e., no '-x'\n");
+        if (command.GetArgumentCount() != 1) {
+            result.GetErrorStream().Printf("error: specify exactly one variable with the '-v' option\n");
             result.SetStatus(eReturnStatusFailed);
             return false;
         }
@@ -1016,7 +1079,8 @@
             if (addr_type == eAddressTypeLoad) {
                 // We're in business.
                 // Find out the size of this variable.
-                size = valobj_sp->GetByteSize();
+                size = m_option_watchpoint.watch_size == 0 ? valobj_sp->GetByteSize()
+                                                           : m_option_watchpoint.watch_size;
             }
         } else {
             const char *error_cstr = error.AsCString(NULL);
@@ -1045,7 +1109,8 @@
         output_stream.EOL();
         result.SetStatus(eReturnStatusSuccessFinishResult);
     } else {
-        result.AppendErrorWithFormat("Watchpoint creation failed.\n");
+        result.AppendErrorWithFormat("Watchpoint creation failed (addr=0x%llx, size=%lu).\n",
+                                     addr, size);
         result.SetStatus(eReturnStatusFailed);
     }
 
diff --git a/source/Commands/CommandObjectWatchpoint.h b/source/Commands/CommandObjectWatchpoint.h
index c61ebf4..ff8d613 100644
--- a/source/Commands/CommandObjectWatchpoint.h
+++ b/source/Commands/CommandObjectWatchpoint.h
@@ -251,6 +251,36 @@
 {
 public:
 
+    class CommandOptions : public OptionGroup
+    {
+    public:
+
+        CommandOptions ();
+
+        virtual
+        ~CommandOptions ();
+
+        virtual uint32_t
+        GetNumDefinitions ();
+        
+        virtual const OptionDefinition*
+        GetDefinitions ();
+        
+        virtual Error
+        SetOptionValue (CommandInterpreter &interpreter,
+                        uint32_t option_idx,
+                        const char *option_value);
+        
+        virtual void
+        OptionParsingStarting (CommandInterpreter &interpreter);
+
+        // Options table: Required for subclasses of Options.
+
+        static OptionDefinition g_option_table[];
+        bool m_do_expression;
+        bool m_do_variable;
+    };
+
     CommandObjectWatchpointSet (CommandInterpreter &interpreter);
 
     virtual
@@ -266,6 +296,7 @@
 private:
     OptionGroupOptions m_option_group;
     OptionGroupWatchpoint m_option_watchpoint;
+    CommandOptions m_command_options;
 };
 
 } // namespace lldb_private
diff --git a/source/Interpreter/CommandObject.cpp b/source/Interpreter/CommandObject.cpp
index 7624e86..5618b6c 100644
--- a/source/Interpreter/CommandObject.cpp
+++ b/source/Interpreter/CommandObject.cpp
@@ -535,15 +535,30 @@
     return false;
 }
 
+static CommandObject::CommandArgumentEntry
+OptSetFiltered(uint32_t opt_set_mask, CommandObject::CommandArgumentEntry &cmd_arg_entry)
+{
+    CommandObject::CommandArgumentEntry ret_val;
+    for (unsigned i = 0; i < cmd_arg_entry.size(); ++i)
+        if (opt_set_mask & cmd_arg_entry[i].arg_opt_set_association)
+            ret_val.push_back(cmd_arg_entry[i]);
+    return ret_val;
+}
+
+// Default parameter value of opt_set_mask is LLDB_OPT_SET_ALL, which means take
+// all the argument data into account.  On rare cases where some argument sticks
+// with certain option sets, this function returns the option set filtered args.
 void
-CommandObject::GetFormattedCommandArguments (Stream &str)
+CommandObject::GetFormattedCommandArguments (Stream &str, uint32_t opt_set_mask)
 {
     int num_args = m_arguments.size();
     for (int i = 0; i < num_args; ++i)
     {
         if (i > 0)
             str.Printf (" ");
-        CommandArgumentEntry arg_entry = m_arguments[i];
+        CommandArgumentEntry arg_entry =
+            opt_set_mask == LLDB_OPT_SET_ALL ? m_arguments[i]
+                                             : OptSetFiltered(opt_set_mask, m_arguments[i]);
         int num_alternatives = arg_entry.size();
 
         if ((num_alternatives == 2)
diff --git a/source/Interpreter/OptionGroupWatchpoint.cpp b/source/Interpreter/OptionGroupWatchpoint.cpp
index 159d00e..4a6afb4 100644
--- a/source/Interpreter/OptionGroupWatchpoint.cpp
+++ b/source/Interpreter/OptionGroupWatchpoint.cpp
@@ -40,8 +40,8 @@
 static OptionDefinition
 g_option_table[] =
 {
-    { LLDB_OPT_SET_1, false, "watch", 'w', required_argument, g_watch_type, 0, eArgTypeWatchType, "Determine how to watch a variable; or, with -x option, its pointee."},
-    { LLDB_OPT_SET_1, false, "xsize", 'x', required_argument, g_watch_size, 0, eArgTypeByteSize, "Number of bytes to use to watch the pointee."}
+    { LLDB_OPT_SET_1, false, "watch", 'w', required_argument, g_watch_type, 0, eArgTypeWatchType, "Specify the type of watching to perform."},
+    { LLDB_OPT_SET_1, false, "xsize", 'x', required_argument, g_watch_size, 0, eArgTypeByteSize, "Number of bytes to use to watch a region."}
 };
 
 
diff --git a/source/Interpreter/Options.cpp b/source/Interpreter/Options.cpp
index 09b17a8..daf631f 100644
--- a/source/Interpreter/Options.cpp
+++ b/source/Interpreter/Options.cpp
@@ -437,6 +437,10 @@
             strm.Printf ("\n");
         strm.Indent (name);
 
+        // Different option sets may require different args.
+        StreamString args_str;
+        cmd->GetFormattedCommandArguments(args_str, opt_set_mask);
+
         // First go through and print all options that take no arguments as
         // a single string. If a command has "-a" "-b" and "-c", this will show
         // up as [-abc]
@@ -556,12 +560,12 @@
             }
         }
         
-        if (arguments_str.GetSize() > 0)
+        if (args_str.GetSize() > 0)
         {
             if (cmd->WantsRawCommandString())
                 strm.Printf(" --");
             
-            strm.Printf (" %s", arguments_str.GetData());
+            strm.Printf (" %s", args_str.GetData());
         }
     }