Refactor parsing of option lists with a raw string suffix.

Summary:
A subset of the LLDB commands follows this command line interface style:
   <command name> [arguments] -- <string suffix>
The parsing code for this interface has been so far been duplicated into the different
command objects which makes it hard to maintain and reuse elsewhere.

This patches improves the situation by adding a OptionsWithRaw class that centralizes
the parsing logic and allows easier testing. The different commands now just call this class to
extract the arguments and the raw suffix from the provided user input.

Reviewers: jingham

Reviewed By: jingham

Subscribers: mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D49106

llvm-svn: 336723

5 files changed, 89 insertions, 228 deletions

diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp
index 584e081c6e8..5e7f272ec22 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -553,42 +553,13 @@ protected:
     ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext();
     m_option_group.NotifyOptionParsingStarting(&exe_ctx);
 
-    const char *remainder = nullptr;
-
-    if (raw_command_line[0] == '-') {
-      // We have some options and these options MUST end with --.
-      const char *end_options = nullptr;
-      const char *s = raw_command_line;
-      while (s && s[0]) {
-        end_options = ::strstr(s, "--");
-        if (end_options) {
-          end_options += 2; // Get past the "--"
-          if (::isspace(end_options[0])) {
-            remainder = end_options;
-            while (::isspace(*remainder))
-              ++remainder;
-            break;
-          }
-        }
-        s = end_options;
-      }
+    OptionsWithRaw args_with_suffix(raw_command_line);
+    const char *remainder = args_with_suffix.GetRawPart().c_str();
 
-      if (end_options) {
-        Args args(
-            llvm::StringRef(raw_command_line, end_options - raw_command_line));
-        if (!ParseOptions(args, result))
-          return false;
-
-        Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx));
-        if (error.Fail()) {
-          result.AppendError(error.AsCString());
-          result.SetStatus(eReturnStatusFailed);
-          return false;
-        }
-      }
-    }
-    if (nullptr == remainder)
-      remainder = raw_command_line;
+    if (args_with_suffix.HasArgs())
+      if (!ParseOptionsAndNotify(args_with_suffix.GetArgs(), result,
+                                 m_option_group, exe_ctx))
+        return false;
 
     llvm::StringRef raw_command_string(remainder);
     Args args(raw_command_string);
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 9fe00f41c56..9373813b8d0 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -514,111 +514,82 @@ bool CommandObjectExpression::DoExecute(const char *command,
   auto exe_ctx = GetCommandInterpreter().GetExecutionContext();
   m_option_group.NotifyOptionParsingStarting(&exe_ctx);
 
-  const char *expr = nullptr;
-
   if (command[0] == '\0') {
     GetMultilineExpression();
     return result.Succeeded();
   }
 
-  if (command[0] == '-') {
-    // We have some options and these options MUST end with --.
-    const char *end_options = nullptr;
-    const char *s = command;
-    while (s && s[0]) {
-      end_options = ::strstr(s, "--");
-      if (end_options) {
-        end_options += 2; // Get past the "--"
-        if (::isspace(end_options[0])) {
-          expr = end_options;
-          while (::isspace(*expr))
-            ++expr;
-          break;
-        }
-      }
-      s = end_options;
-    }
-
-    if (end_options) {
-      Args args(llvm::StringRef(command, end_options - command));
-      if (!ParseOptions(args, result))
-        return false;
-
-      Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx));
-      if (error.Fail()) {
-        result.AppendError(error.AsCString());
-        result.SetStatus(eReturnStatusFailed);
-        return false;
-      }
-
-      if (m_repl_option.GetOptionValue().GetCurrentValue()) {
-        Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
-        if (target) {
-          // Drop into REPL
-          m_expr_lines.clear();
-          m_expr_line_count = 0;
-
-          Debugger &debugger = target->GetDebugger();
-
-          // Check if the LLDB command interpreter is sitting on top of a REPL
-          // that launched it...
-          if (debugger.CheckTopIOHandlerTypes(
-                  IOHandler::Type::CommandInterpreter, IOHandler::Type::REPL)) {
-            // the LLDB command interpreter is sitting on top of a REPL that
-            // launched it, so just say the command interpreter is done and
-            // fall back to the existing REPL
-            m_interpreter.GetIOHandler(false)->SetIsDone(true);
-          } else {
-            // We are launching the REPL on top of the current LLDB command
-            // interpreter, so just push one
-            bool initialize = false;
-            Status repl_error;
-            REPLSP repl_sp(target->GetREPL(
-                repl_error, m_command_options.language, nullptr, false));
-
-            if (!repl_sp) {
-              initialize = true;
-              repl_sp = target->GetREPL(repl_error, m_command_options.language,
-                                        nullptr, true);
-              if (!repl_error.Success()) {
-                result.SetError(repl_error);
-                return result.Succeeded();
-              }
+  OptionsWithRaw args(command);
+  const char *expr = args.GetRawPart().c_str();
+
+  if (args.HasArgs()) {
+    if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, exe_ctx))
+      return false;
+
+    if (m_repl_option.GetOptionValue().GetCurrentValue()) {
+      Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
+      if (target) {
+        // Drop into REPL
+        m_expr_lines.clear();
+        m_expr_line_count = 0;
+
+        Debugger &debugger = target->GetDebugger();
+
+        // Check if the LLDB command interpreter is sitting on top of a REPL
+        // that launched it...
+        if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
+                                            IOHandler::Type::REPL)) {
+          // the LLDB command interpreter is sitting on top of a REPL that
+          // launched it, so just say the command interpreter is done and
+          // fall back to the existing REPL
+          m_interpreter.GetIOHandler(false)->SetIsDone(true);
+        } else {
+          // We are launching the REPL on top of the current LLDB command
+          // interpreter, so just push one
+          bool initialize = false;
+          Status repl_error;
+          REPLSP repl_sp(target->GetREPL(repl_error, m_command_options.language,
+                                         nullptr, false));
+
+          if (!repl_sp) {
+            initialize = true;
+            repl_sp = target->GetREPL(repl_error, m_command_options.language,
+                                      nullptr, true);
+            if (!repl_error.Success()) {
+              result.SetError(repl_error);
+              return result.Succeeded();
             }
+          }
 
-            if (repl_sp) {
-              if (initialize) {
-                repl_sp->SetCommandOptions(m_command_options);
-                repl_sp->SetFormatOptions(m_format_options);
-                repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
-              }
+          if (repl_sp) {
+            if (initialize) {
+              repl_sp->SetCommandOptions(m_command_options);
+              repl_sp->SetFormatOptions(m_format_options);
+              repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
+            }
 
-              IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
+            IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
 
-              io_handler_sp->SetIsDone(false);
+            io_handler_sp->SetIsDone(false);
 
-              debugger.PushIOHandler(io_handler_sp);
-            } else {
-              repl_error.SetErrorStringWithFormat(
-                  "Couldn't create a REPL for %s",
-                  Language::GetNameForLanguageType(m_command_options.language));
-              result.SetError(repl_error);
-              return result.Succeeded();
-            }
+            debugger.PushIOHandler(io_handler_sp);
+          } else {
+            repl_error.SetErrorStringWithFormat(
+                "Couldn't create a REPL for %s",
+                Language::GetNameForLanguageType(m_command_options.language));
+            result.SetError(repl_error);
+            return result.Succeeded();
           }
         }
       }
-      // No expression following options
-      else if (expr == nullptr || expr[0] == '\0') {
-        GetMultilineExpression();
-        return result.Succeeded();
-      }
+    }
+    // No expression following options
+    else if (expr[0] == '\0') {
+      GetMultilineExpression();
+      return result.Succeeded();
     }
   }
 
-  if (expr == nullptr)
-    expr = command;
-
   Target *target = GetSelectedOrDummyTarget();
   if (EvaluateExpression(expr, &(result.GetOutputStream()),
                          &(result.GetErrorStream()), &result)) {
@@ -629,13 +600,12 @@ bool CommandObjectExpression::DoExecute(const char *command,
       // for expr???)
       // If we can it would be nice to show that.
       std::string fixed_command("expression ");
-      if (expr == command)
-        fixed_command.append(m_fixed_expression);
-      else {
+      if (args.HasArgs()) {
         // Add in any options that might have been in the original command:
-        fixed_command.append(command, expr - command);
+        fixed_command.append(args.GetArgStringWithDelimiter());
+        fixed_command.append(m_fixed_expression);
+      } else
         fixed_command.append(m_fixed_expression);
-      }
       history.AppendString(fixed_command);
     }
     // Increment statistics to record this expression evaluation success.
diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp b/lldb/source/Commands/CommandObjectPlatform.cpp
index 118f78e359e..417d0ebddd5 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1747,7 +1747,6 @@ public:
     ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext();
     m_options.NotifyOptionParsingStarting(&exe_ctx);
 
-    const char *expr = nullptr;
 
     // Print out an usage syntax on an empty command line.
     if (raw_command_line[0] == '\0') {
@@ -1755,34 +1754,12 @@ public:
       return true;
     }
 
-    if (raw_command_line[0] == '-') {
-      // We have some options and these options MUST end with --.
-      const char *end_options = nullptr;
-      const char *s = raw_command_line;
-      while (s && s[0]) {
-        end_options = ::strstr(s, "--");
-        if (end_options) {
-          end_options += 2; // Get past the "--"
-          if (::isspace(end_options[0])) {
-            expr = end_options;
-            while (::isspace(*expr))
-              ++expr;
-            break;
-          }
-        }
-        s = end_options;
-      }
+    OptionsWithRaw args(raw_command_line);
+    const char *expr = args.GetRawPart().c_str();
 
-      if (end_options) {
-        Args args(
-            llvm::StringRef(raw_command_line, end_options - raw_command_line));
-        if (!ParseOptions(args, result))
-          return false;
-      }
-    }
-
-    if (expr == nullptr)
-      expr = raw_command_line;
+    if (args.HasArgs())
+      if (!ParseOptions(args.GetArgs(), result))
+        return false;
 
     PlatformSP platform_sp(
         m_interpreter.GetDebugger().GetPlatformList().GetSelectedPlatform());
diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index 73393076208..1ff19d1853c 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -2873,42 +2873,13 @@ public:
     auto exe_ctx = GetCommandInterpreter().GetExecutionContext();
     m_option_group.NotifyOptionParsingStarting(&exe_ctx);
 
-    const char *name_of_type = nullptr;
-
-    if (raw_command_line[0] == '-') {
-      // We have some options and these options MUST end with --.
-      const char *end_options = nullptr;
-      const char *s = raw_command_line;
-      while (s && s[0]) {
-        end_options = ::strstr(s, "--");
-        if (end_options) {
-          end_options += 2; // Get past the "--"
-          if (::isspace(end_options[0])) {
-            name_of_type = end_options;
-            while (::isspace(*name_of_type))
-              ++name_of_type;
-            break;
-          }
-        }
-        s = end_options;
-      }
+    OptionsWithRaw args(raw_command_line);
+    const char *name_of_type = args.GetRawPart().c_str();
 
-      if (end_options) {
-        Args args(
-            llvm::StringRef(raw_command_line, end_options - raw_command_line));
-        if (!ParseOptions(args, result))
-          return false;
-
-        Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx));
-        if (error.Fail()) {
-          result.AppendError(error.AsCString());
-          result.SetStatus(eReturnStatusFailed);
-          return false;
-        }
-      }
-    }
-    if (nullptr == name_of_type)
-      name_of_type = raw_command_line;
+    if (args.HasArgs())
+      if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group,
+                                 exe_ctx))
+        return false;
 
     // TargetSP
     // target_sp(GetCommandInterpreter().GetDebugger().GetSelectedTarget());
diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 7217c440cbc..14d1083ca31 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -1035,46 +1035,18 @@ protected:
     Target *target = m_interpreter.GetDebugger().GetSelectedTarget().get();
     StackFrame *frame = m_exe_ctx.GetFramePtr();
 
-    Args command(raw_command);
-    const char *expr = nullptr;
-    if (raw_command[0] == '-') {
-      // We have some options and these options MUST end with --.
-      const char *end_options = nullptr;
-      const char *s = raw_command;
-      while (s && s[0]) {
-        end_options = ::strstr(s, "--");
-        if (end_options) {
-          end_options += 2; // Get past the "--"
-          if (::isspace(end_options[0])) {
-            expr = end_options;
-            while (::isspace(*expr))
-              ++expr;
-            break;
-          }
-        }
-        s = end_options;
-      }
+    OptionsWithRaw args(raw_command);
 
-      if (end_options) {
-        Args args(llvm::StringRef(raw_command, end_options - raw_command));
-        if (!ParseOptions(args, result))
-          return false;
-
-        Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx));
-        if (error.Fail()) {
-          result.AppendError(error.AsCString());
-          result.SetStatus(eReturnStatusFailed);
-          return false;
-        }
-      }
-    }
+    llvm::StringRef expr = args.GetRawPart();
 
-    if (expr == nullptr)
-      expr = 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 (command.GetArgumentCount() == 0) {
+    if (llvm::StringRef(raw_command).trim().empty()) {
       result.GetErrorStream().Printf("error: required argument missing; "
                                      "specify an expression to evaulate into "
                                      "the address to watch for\n");
@@ -1107,7 +1079,7 @@ protected:
     if (expr_result != eExpressionCompleted) {
       result.GetErrorStream().Printf(
           "error: expression evaluation of address to watch failed\n");
-      result.GetErrorStream().Printf("expression evaluated: %s\n", expr);
+      result.GetErrorStream() << "expression evaluated: \n" << expr << "\n";
       result.SetStatus(eReturnStatusFailed);
       return false;
     }