[Lldb-commits] [lldb] r336723 - Refactor parsing of option lists with a raw string suffix.

Raphael Isemann via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 10 13:17:39 PDT 2018


Author: teemperor
Date: Tue Jul 10 13:17:38 2018
New Revision: 336723

URL: http://llvm.org/viewvc/llvm-project?rev=336723&view=rev
Log:
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

Added:
    lldb/trunk/unittests/Utility/OptionsWithRawTest.cpp
Modified:
    lldb/trunk/include/lldb/Interpreter/CommandObject.h
    lldb/trunk/include/lldb/Utility/Args.h
    lldb/trunk/source/Commands/CommandObjectCommands.cpp
    lldb/trunk/source/Commands/CommandObjectExpression.cpp
    lldb/trunk/source/Commands/CommandObjectPlatform.cpp
    lldb/trunk/source/Commands/CommandObjectType.cpp
    lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp
    lldb/trunk/source/Interpreter/CommandObject.cpp
    lldb/trunk/source/Interpreter/Options.cpp
    lldb/trunk/source/Utility/Args.cpp
    lldb/trunk/unittests/Utility/CMakeLists.txt

Modified: lldb/trunk/include/lldb/Interpreter/CommandObject.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandObject.h?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Interpreter/CommandObject.h (original)
+++ lldb/trunk/include/lldb/Interpreter/CommandObject.h Tue Jul 10 13:17:38 2018
@@ -337,6 +337,10 @@ public:
                        CommandReturnObject &result) = 0;
 
 protected:
+  bool ParseOptionsAndNotify(Args &args, CommandReturnObject &result,
+                             OptionGroupOptions &group_options,
+                             ExecutionContext &exe_ctx);
+
   virtual const char *GetInvalidTargetDescription() {
     return "invalid target, create a target using the 'target create' command";
   }

Modified: lldb/trunk/include/lldb/Utility/Args.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Args.h?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Utility/Args.h (original)
+++ lldb/trunk/include/lldb/Utility/Args.h Tue Jul 10 13:17:38 2018
@@ -48,6 +48,11 @@ public:
     llvm::StringRef ref;
     char quote;
     const char *c_str() const { return ptr.get(); }
+
+    //------------------------------------------------------------------
+    /// Returns true if this argument was quoted in any way.
+    //------------------------------------------------------------------
+    bool IsQuoted() const { return quote != '\0'; }
   };
 
   //------------------------------------------------------------------
@@ -353,6 +358,106 @@ private:
   std::vector<char *> m_argv;
 };
 
+//----------------------------------------------------------------------
+/// @class OptionsWithRaw Args.h "lldb/Utility/Args.h"
+/// A pair of an option list with a 'raw' string as a suffix.
+///
+/// This class works similar to Args, but handles the case where we have a
+/// trailing string that shouldn't be interpreted as a list of arguments but
+/// preserved as is. It is also only useful for handling command line options
+/// (e.g. '-foo bar -i0') that start with a dash.
+///
+/// The leading option list is optional. If the first non-space character
+/// in the string starts with a dash, and the string contains an argument
+/// that is an unquoted double dash (' -- '), then everything up to the double
+/// dash is parsed as a list of arguments. Everything after the double dash
+/// is interpreted as the raw suffix string. Note that the space behind the
+/// double dash is not part of the raw suffix.
+///
+/// All strings not matching the above format as considered to be just a raw
+/// string without any options.
+///
+/// @see Args
+//----------------------------------------------------------------------
+class OptionsWithRaw {
+public:
+  //------------------------------------------------------------------
+  /// Parse the given string as a list of optional arguments with a raw suffix.
+  ///
+  /// See the class description for a description of the input format.
+  ///
+  /// @param[in] argument_string
+  ///     The string that should be parsed.
+  //------------------------------------------------------------------
+  explicit OptionsWithRaw(llvm::StringRef argument_string);
+
+  //------------------------------------------------------------------
+  /// Returns true if there are any arguments before the raw suffix.
+  //------------------------------------------------------------------
+  bool HasArgs() const { return m_has_args; }
+
+  //------------------------------------------------------------------
+  /// Returns the list of arguments.
+  ///
+  /// You can only call this method if HasArgs returns true.
+  //------------------------------------------------------------------
+  Args &GetArgs() {
+    assert(m_has_args);
+    return m_args;
+  }
+
+  //------------------------------------------------------------------
+  /// Returns the list of arguments.
+  ///
+  /// You can only call this method if HasArgs returns true.
+  //------------------------------------------------------------------
+  const Args &GetArgs() const {
+    assert(m_has_args);
+    return m_args;
+  }
+
+  //------------------------------------------------------------------
+  /// Returns the part of the input string that was used for parsing the
+  /// argument list. This string also includes the double dash that is used
+  /// for separating the argument list from the suffix.
+  ///
+  /// You can only call this method if HasArgs returns true.
+  //------------------------------------------------------------------
+  llvm::StringRef GetArgStringWithDelimiter() const {
+    assert(m_has_args);
+    return m_arg_string_with_delimiter;
+  }
+
+  //------------------------------------------------------------------
+  /// Returns the part of the input string that was used for parsing the
+  /// argument list.
+  ///
+  /// You can only call this method if HasArgs returns true.
+  //------------------------------------------------------------------
+  llvm::StringRef GetArgString() const {
+    assert(m_has_args);
+    return m_arg_string;
+  }
+
+  //------------------------------------------------------------------
+  /// Returns the raw suffix part of the parsed string.
+  //------------------------------------------------------------------
+  const std::string &GetRawPart() const { return m_suffix; }
+
+private:
+  void SetFromString(llvm::StringRef arg_string);
+
+  /// Keeps track if we have parsed and stored any arguments.
+  bool m_has_args = false;
+  Args m_args;
+  llvm::StringRef m_arg_string;
+  llvm::StringRef m_arg_string_with_delimiter;
+
+  // FIXME: This should be a StringRef, but some of the calling code expect a
+  // C string here so only a real std::string is possible.
+  std::string m_suffix;
+};
+
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_ARGS_H

Modified: lldb/trunk/source/Commands/CommandObjectCommands.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectCommands.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectCommands.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectCommands.cpp Tue Jul 10 13:17:38 2018
@@ -553,42 +553,13 @@ protected:
     ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext();
     m_option_group.NotifyOptionParsingStarting(&exe_ctx);
 
-    const char *remainder = nullptr;
+    OptionsWithRaw args_with_suffix(raw_command_line);
+    const char *remainder = args_with_suffix.GetRawPart().c_str();
 
-    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;
-      }
-
-      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);

Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Tue Jul 10 13:17:38 2018
@@ -514,111 +514,82 @@ bool CommandObjectExpression::DoExecute(
   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;
-      }
+  OptionsWithRaw args(command);
+  const char *expr = args.GetRawPart().c_str();
 
-      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);
-              }
-
-              IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
-
-              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));
+  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);
+            }
+
+            IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
+
+            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();
+          }
         }
       }
-      // 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(
       // 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.

Modified: lldb/trunk/source/Commands/CommandObjectPlatform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectPlatform.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectPlatform.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectPlatform.cpp Tue Jul 10 13:17:38 2018
@@ -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());

Modified: lldb/trunk/source/Commands/CommandObjectType.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectType.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectType.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectType.cpp Tue Jul 10 13:17:38 2018
@@ -2873,42 +2873,13 @@ public:
     auto exe_ctx = GetCommandInterpreter().GetExecutionContext();
     m_option_group.NotifyOptionParsingStarting(&exe_ctx);
 
-    const char *name_of_type = nullptr;
+    OptionsWithRaw args(raw_command_line);
+    const char *name_of_type = args.GetRawPart().c_str();
 
-    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;
-      }
-
-      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());

Modified: lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectWatchpoint.cpp Tue Jul 10 13:17:38 2018
@@ -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;
-      }
-
-      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;
-        }
-      }
-    }
+    OptionsWithRaw args(raw_command);
+
+    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;
     }

Modified: lldb/trunk/source/Interpreter/CommandObject.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandObject.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/CommandObject.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandObject.cpp Tue Jul 10 13:17:38 2018
@@ -331,6 +331,22 @@ bool CommandObject::HelpTextContainsWord
   return found_word;
 }
 
+bool CommandObject::ParseOptionsAndNotify(Args &args,
+                                          CommandReturnObject &result,
+                                          OptionGroupOptions &group_options,
+                                          ExecutionContext &exe_ctx) {
+  if (!ParseOptions(args, result))
+    return false;
+
+  Status error(group_options.NotifyOptionParsingFinished(&exe_ctx));
+  if (error.Fail()) {
+    result.AppendError(error.AsCString());
+    result.SetStatus(eReturnStatusFailed);
+    return false;
+  }
+  return true;
+}
+
 int CommandObject::GetNumArgumentEntries() { return m_arguments.size(); }
 
 CommandObject::CommandArgumentEntry *

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Tue Jul 10 13:17:38 2018
@@ -1334,7 +1334,7 @@ OptionElementVector Options::ParseForCom
   const Args::ArgEntry &cursor = args[cursor_index];
   if ((static_cast<int32_t>(dash_dash_pos) == -1 ||
        cursor_index < dash_dash_pos) &&
-      cursor.quote == '\0' && cursor.ref == "-") {
+      !cursor.IsQuoted() && cursor.ref == "-") {
     option_element_vector.push_back(
         OptionArgElement(OptionArgElement::eBareDash, cursor_index,
                          OptionArgElement::eBareDash));

Modified: lldb/trunk/source/Utility/Args.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Args.cpp?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/source/Utility/Args.cpp (original)
+++ lldb/trunk/source/Utility/Args.cpp Tue Jul 10 13:17:38 2018
@@ -66,6 +66,13 @@ static size_t ArgvToArgc(const char **ar
   return count;
 }
 
+// Trims all whitespace that can separate command line arguments from the left
+// side of the string.
+static llvm::StringRef ltrimForArgs(llvm::StringRef str) {
+  static const char *k_space_separators = " \t";
+  return str.ltrim(k_space_separators);
+}
+
 // A helper function for SetCommandString. Parses a single argument from the
 // command string, processing quotes and backslashes in a shell-like manner.
 // The function returns a tuple consisting of the parsed argument, the quote
@@ -237,15 +244,14 @@ void Args::SetCommandString(llvm::String
   Clear();
   m_argv.clear();
 
-  static const char *k_space_separators = " \t";
-  command = command.ltrim(k_space_separators);
+  command = ltrimForArgs(command);
   std::string arg;
   char quote;
   while (!command.empty()) {
     std::tie(arg, quote, command) = ParseSingleArgument(command);
     m_entries.emplace_back(arg, quote);
     m_argv.push_back(m_entries.back().data());
-    command = command.ltrim(k_space_separators);
+    command = ltrimForArgs(command);
   }
   m_argv.push_back(nullptr);
 }
@@ -654,3 +660,67 @@ std::string Args::EscapeLLDBCommandArgum
   }
   return res;
 }
+
+OptionsWithRaw::OptionsWithRaw(llvm::StringRef arg_string) {
+  SetFromString(arg_string);
+}
+
+void OptionsWithRaw::SetFromString(llvm::StringRef arg_string) {
+  const llvm::StringRef original_args = arg_string;
+
+  arg_string = ltrimForArgs(arg_string);
+  std::string arg;
+  char quote;
+
+  // If the string doesn't start with a dash, we just have no options and just
+  // a raw part.
+  if (!arg_string.startswith("-")) {
+    m_suffix = original_args;
+    return;
+  }
+
+  bool found_suffix = false;
+
+  while (!arg_string.empty()) {
+    // The length of the prefix before parsing.
+    std::size_t prev_prefix_length = original_args.size() - arg_string.size();
+
+    // Parse the next argument from the remaining string.
+    std::tie(arg, quote, arg_string) = ParseSingleArgument(arg_string);
+
+    // If we get an unquoted '--' argument, then we reached the suffix part
+    // of the command.
+    Args::ArgEntry entry(arg, quote);
+    if (!entry.IsQuoted() && arg == "--") {
+      // The remaining line is the raw suffix, and the line we parsed so far
+      // needs to be interpreted as arguments.
+      m_has_args = true;
+      m_suffix = arg_string;
+      found_suffix = true;
+
+      // The length of the prefix after parsing.
+      std::size_t prefix_length = original_args.size() - arg_string.size();
+
+      // Take the string we know contains all the arguments and actually parse
+      // it as proper arguments.
+      llvm::StringRef prefix = original_args.take_front(prev_prefix_length);
+      m_args = Args(prefix);
+      m_arg_string = prefix;
+
+      // We also record the part of the string that contains the arguments plus
+      // the delimiter.
+      m_arg_string_with_delimiter = original_args.take_front(prefix_length);
+
+      // As the rest of the string became the raw suffix, we are done here.
+      break;
+    }
+
+    arg_string = ltrimForArgs(arg_string);
+  }
+
+  // If we didn't find a suffix delimiter, the whole string is the raw suffix.
+  if (!found_suffix) {
+    found_suffix = true;
+    m_suffix = original_args;
+  }
+}

Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=336723&r1=336722&r2=336723&view=diff
==============================================================================
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Tue Jul 10 13:17:38 2018
@@ -1,5 +1,6 @@
 add_lldb_unittest(UtilityTests
   ArgsTest.cpp
+  OptionsWithRawTest.cpp
   ArchSpecTest.cpp
   CleanUpTest.cpp
   ConstStringTest.cpp

Added: lldb/trunk/unittests/Utility/OptionsWithRawTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/OptionsWithRawTest.cpp?rev=336723&view=auto
==============================================================================
--- lldb/trunk/unittests/Utility/OptionsWithRawTest.cpp (added)
+++ lldb/trunk/unittests/Utility/OptionsWithRawTest.cpp Tue Jul 10 13:17:38 2018
@@ -0,0 +1,183 @@
+//===-- OptionsWithRawTest.cpp ----------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/Args.h"
+#include "lldb/Utility/StringList.h"
+
+using namespace lldb_private;
+
+TEST(OptionsWithRawTest, EmptyInput) {
+  // An empty string is just an empty suffix without any arguments.
+  OptionsWithRaw args("");
+  ASSERT_FALSE(args.HasArgs());
+  ASSERT_STREQ(args.GetRawPart().c_str(), "");
+}
+
+TEST(OptionsWithRawTest, SingleWhitespaceInput) {
+  // Only whitespace is just a suffix.
+  OptionsWithRaw args(" ");
+  ASSERT_FALSE(args.HasArgs());
+  ASSERT_STREQ(args.GetRawPart().c_str(), " ");
+}
+
+TEST(OptionsWithRawTest, WhitespaceInput) {
+  // Only whitespace is just a suffix.
+  OptionsWithRaw args("  ");
+  ASSERT_FALSE(args.HasArgs());
+  ASSERT_STREQ(args.GetRawPart().c_str(), "  ");
+}
+
+TEST(OptionsWithRawTest, ArgsButNoDelimiter) {
+  // This counts as a suffix because there is no -- at the end.
+  OptionsWithRaw args("-foo bar");
+  ASSERT_FALSE(args.HasArgs());
+  ASSERT_STREQ(args.GetRawPart().c_str(), "-foo bar");
+}
+
+TEST(OptionsWithRawTest, ArgsButNoLeadingDash) {
+  // No leading dash means we have no arguments.
+  OptionsWithRaw args("foo bar --");
+  ASSERT_FALSE(args.HasArgs());
+  ASSERT_STREQ(args.GetRawPart().c_str(), "foo bar --");
+}
+
+TEST(OptionsWithRawTest, QuotedSuffix) {
+  // We need to have a way to escape the -- to make it usable as an argument.
+  OptionsWithRaw args("-foo \"--\" bar");
+  ASSERT_FALSE(args.HasArgs());
+  ASSERT_STREQ(args.GetRawPart().c_str(), "-foo \"--\" bar");
+}
+
+TEST(OptionsWithRawTest, EmptySuffix) {
+  // An empty suffix with arguments.
+  OptionsWithRaw args("-foo --");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo --");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "");
+}
+
+TEST(OptionsWithRawTest, EmptySuffixSingleWhitespace) {
+  // A single whitespace also countas as an empty suffix (because that usually
+  // separates the suffix from the double dash.
+  OptionsWithRaw args("-foo -- ");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "");
+}
+
+TEST(OptionsWithRawTest, WhitespaceSuffix) {
+  // A single whtiespace character as a suffix.
+  OptionsWithRaw args("-foo --  ");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), " ");
+}
+
+TEST(OptionsWithRawTest, LeadingSpaceArgs) {
+  // Whitespace before the first dash needs to be ignored.
+  OptionsWithRaw args(" -foo -- bar");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), " -foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), " -foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "bar");
+}
+
+TEST(OptionsWithRawTest, SingleWordSuffix) {
+  // A single word as a suffix.
+  OptionsWithRaw args("-foo -- bar");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "bar");
+}
+
+TEST(OptionsWithRawTest, MultiWordSuffix) {
+  // Multiple words as a suffix.
+  OptionsWithRaw args("-foo -- bar baz");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "bar baz");
+}
+
+TEST(OptionsWithRawTest, UnterminatedQuote) {
+  // A quote character in the suffix shouldn't influence the parsing.
+  OptionsWithRaw args("-foo -- bar \" ");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "bar \" ");
+}
+
+TEST(OptionsWithRawTest, TerminatedQuote) {
+  // A part of the suffix is quoted, which shouldn't influence the parsing.
+  OptionsWithRaw args("-foo -- bar \"a\" ");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "-foo ");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(1u, ref.size());
+  EXPECT_STREQ("-foo", ref[0]);
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "bar \"a\" ");
+}
+
+TEST(OptionsWithRawTest, EmptyArgsOnlySuffix) {
+  // Empty argument list, but we have a suffix.
+  OptionsWithRaw args("-- bar");
+  ASSERT_TRUE(args.HasArgs());
+  ASSERT_EQ(args.GetArgString(), "");
+  ASSERT_EQ(args.GetArgStringWithDelimiter(), "-- ");
+
+  auto ref = args.GetArgs().GetArgumentArrayRef();
+  ASSERT_EQ(0u, ref.size());
+
+  ASSERT_STREQ(args.GetRawPart().c_str(), "bar");
+}




More information about the lldb-commits mailing list