[Lldb-commits] [lldb] r327110 - Move option parsing out of the Args class

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Fri Mar 9 02:39:40 PST 2018


Author: labath
Date: Fri Mar  9 02:39:40 2018
New Revision: 327110

URL: http://llvm.org/viewvc/llvm-project?rev=327110&view=rev
Log:
Move option parsing out of the Args class

Summary:
The args class is used in plenty of places (a lot of them in the lower lldb
layers) for representing a list of arguments, and most of these places don't
care about option parsing. Moving the option parsing out of the class removes
the largest external dependency (there are a couple more, but these are in
static functions), and brings us closer to being able to move it to the
Utility module).

The new home for these functions is the Options class, which was already used
as an argument to the parse calls, so this just inverts the dependency between
the two.

The functions are themselves are mainly just copied -- the biggest functional
change I've made to them is to avoid modifying the input Args argument (getopt
likes to permute the argument vector), as it was weird to have another class
reorder the entries in Args class. So now the functions don't modify the input
arguments, and (for those where it makes sense) return a new Args vector
instead. I've also made the addition of a "fake arg0" (required for getopt
compatibility) an implementation detail rather than a part of interface.

While doing that I noticed that ParseForCompletion function was recording the
option indexes in the shuffled vector, but then the consumer was looking up the
entries in the unshuffled one. This manifested itself as us not being able to
complete "watchpoint set variable foo --" (because getopt would move "foo" to
the end). Surprisingly all other completions (e.g. "watchpoint set variable foo
--w") were not affected by this. However, I couldn't find a comprehensive test
for command argument completion, so I consolidated the existing tests and added
a bunch of new ones.

Reviewers: davide, jingham, zturner

Subscribers: lldb-commits

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

Modified:
    lldb/trunk/include/lldb/Interpreter/Args.h
    lldb/trunk/include/lldb/Interpreter/Options.h
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
    lldb/trunk/source/Commands/CommandObjectSettings.cpp
    lldb/trunk/source/Interpreter/Args.cpp
    lldb/trunk/source/Interpreter/CommandAlias.cpp
    lldb/trunk/source/Interpreter/CommandObject.cpp
    lldb/trunk/source/Interpreter/Options.cpp
    lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp

Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Fri Mar  9 02:39:40 2018
@@ -28,8 +28,6 @@
 
 namespace lldb_private {
 
-struct Option;
-
 typedef std::vector<std::tuple<std::string, int, std::string>> OptionArgVector;
 typedef std::shared_ptr<OptionArgVector> OptionArgVectorSP;
 
@@ -161,10 +159,10 @@ public:
   llvm::ArrayRef<ArgEntry> entries() const { return m_entries; }
   char GetArgumentQuoteCharAtIndex(size_t idx) const;
 
-  std::vector<ArgEntry>::const_iterator begin() const {
-    return m_entries.begin();
-  }
-  std::vector<ArgEntry>::const_iterator end() const { return m_entries.end(); }
+  using const_iterator = std::vector<ArgEntry>::const_iterator;
+
+  const_iterator begin() const { return m_entries.begin(); }
+  const_iterator end() const { return m_entries.end(); }
 
   size_t size() const { return GetArgumentCount(); }
   const ArgEntry &operator[](size_t n) const { return m_entries[n]; }
@@ -310,44 +308,6 @@ public:
   void Unshift(llvm::StringRef arg_str, char quote_char = '\0');
 
   //------------------------------------------------------------------
-  /// Parse the arguments in the contained arguments.
-  ///
-  /// The arguments that are consumed by the argument parsing process
-  /// will be removed from the argument vector. The arguments that
-  /// get processed start at the second argument. The first argument
-  /// is assumed to be the command and will not be touched.
-  ///
-  /// param[in] platform_sp
-  ///   The platform used for option validation.  This is necessary
-  ///   because an empty execution_context is not enough to get us
-  ///   to a reasonable platform.  If the platform isn't given,
-  ///   we'll try to get it from the execution context.  If we can't
-  ///   get it from the execution context, we'll skip validation.
-  ///
-  /// param[in] require_validation
-  ///   When true, it will fail option parsing if validation could
-  ///   not occur due to not having a platform.
-  ///
-  /// @see class Options
-  //------------------------------------------------------------------
-  Status ParseOptions(Options &options, ExecutionContext *execution_context,
-                      lldb::PlatformSP platform_sp, bool require_validation);
-
-  bool IsPositionalArgument(const char *arg);
-
-  // The following works almost identically to ParseOptions, except that no
-  // option is required to have arguments, and it builds up the
-  // option_arg_vector as it parses the options.
-
-  std::string ParseAliasOptions(Options &options, CommandReturnObject &result,
-                                OptionArgVector *option_arg_vector,
-                                llvm::StringRef raw_input_line);
-
-  void ParseArgsForCompletion(Options &options,
-                              OptionElementVector &option_element_vector,
-                              uint32_t cursor_index);
-
-  //------------------------------------------------------------------
   // Clear the arguments.
   //
   // For re-setting or blanking out the list of arguments.
@@ -441,13 +401,8 @@ public:
                                                char quote_char);
 
 private:
-  size_t FindArgumentIndexForOption(Option *long_options,
-                                    int long_options_index) const;
-
   std::vector<ArgEntry> m_entries;
   std::vector<char *> m_argv;
-
-  void UpdateArgsAfterOptionParsing();
 };
 
 } // namespace lldb_private

Modified: lldb/trunk/include/lldb/Interpreter/Options.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Options.h?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Interpreter/Options.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Options.h Fri Mar  9 02:39:40 2018
@@ -25,6 +25,8 @@
 
 namespace lldb_private {
 
+struct Option;
+
 static inline bool isprint8(int ch) {
   if (ch & 0xffffff00u)
     return false;
@@ -36,10 +38,8 @@ static inline bool isprint8(int ch) {
 /// @brief A command line option parsing protocol class.
 ///
 /// Options is designed to be subclassed to contain all needed
-/// options for a given command. The options can be parsed by calling:
-/// \code
-///     Status Args::ParseOptions (Options &);
-/// \endcode
+/// options for a given command. The options can be parsed by calling the Parse
+/// function.
 ///
 /// The options are specified using the format defined for the libc
 /// options parsing function getopt_long_only:
@@ -49,74 +49,6 @@ static inline bool isprint8(int ch) {
 ///     *optstring, const struct option *longopts, int *longindex);
 /// \endcode
 ///
-/// Example code:
-/// \code
-///     #include <getopt.h>
-///     #include <string>
-///
-///     class CommandOptions : public Options
-///     {
-///     public:
-///         virtual struct option *
-///         GetLongOptions() {
-///             return g_options;
-///         }
-///
-///         virtual Status
-///         SetOptionValue (uint32_t option_idx, int option_val, const char
-///         *option_arg)
-///         {
-///             Status error;
-///             switch (option_val)
-///             {
-///             case 'g': debug = true; break;
-///             case 'v': verbose = true; break;
-///             case 'l': log_file = option_arg; break;
-///             case 'f': log_flags = strtoull(option_arg, nullptr, 0); break;
-///             default:
-///                 error.SetErrorStringWithFormat("unrecognized short option
-///                 %c", option_val);
-///                 break;
-///             }
-///
-///             return error;
-///         }
-///
-///         CommandOptions (CommandInterpreter &interpreter) : debug (true),
-///         verbose (false), log_file (), log_flags (0)
-///         {}
-///
-///         bool debug;
-///         bool verbose;
-///         std::string log_file;
-///         uint32_t log_flags;
-///
-///         static struct option g_options[];
-///
-///     };
-///
-///     struct option CommandOptions::g_options[] =
-///     {
-///         { "debug",              no_argument,        nullptr,   'g' },
-///         { "log-file",           required_argument,  nullptr,   'l' },
-///         { "log-flags",          required_argument,  nullptr,   'f' },
-///         { "verbose",            no_argument,        nullptr,   'v' },
-///         { nullptr,              0,                  nullptr,   0   }
-///     };
-///
-///     int main (int argc, const char **argv, const char **envp)
-///     {
-///         CommandOptions options;
-///         Args main_command;
-///         main_command.SetArguments(argc, argv, false);
-///         main_command.ParseOptions(options);
-///
-///         if (options.verbose)
-///         {
-///             std::cout << "verbose is on" << std::endl;
-///         }
-///     }
-/// \endcode
 //----------------------------------------------------------------------
 class Options {
 public:
@@ -171,6 +103,36 @@ public:
   // prone and subclasses shouldn't have to do it.
   void NotifyOptionParsingStarting(ExecutionContext *execution_context);
 
+  //------------------------------------------------------------------
+  /// Parse the provided arguments.
+  ///
+  /// The parsed options are set via calls to SetOptionValue. In case of a
+  /// successful parse, the function returns a copy of the input arguments with
+  /// the parsed options removed. Otherwise, it returns an error.
+  ///
+  /// param[in] platform_sp
+  ///   The platform used for option validation.  This is necessary
+  ///   because an empty execution_context is not enough to get us
+  ///   to a reasonable platform.  If the platform isn't given,
+  ///   we'll try to get it from the execution context.  If we can't
+  ///   get it from the execution context, we'll skip validation.
+  ///
+  /// param[in] require_validation
+  ///   When true, it will fail option parsing if validation could
+  ///   not occur due to not having a platform.
+  //------------------------------------------------------------------
+  llvm::Expected<Args> Parse(const Args &args,
+                             ExecutionContext *execution_context,
+                             lldb::PlatformSP platform_sp,
+                             bool require_validation);
+
+  llvm::Expected<Args> ParseAlias(const Args &args,
+                                  OptionArgVector *option_arg_vector,
+                                  std::string &input_line);
+
+  OptionElementVector ParseForCompletion(const Args &args,
+                                         uint32_t cursor_index);
+
   Status NotifyOptionParsingFinished(ExecutionContext *execution_context);
 
   //------------------------------------------------------------------

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py Fri Mar  9 02:39:40 2018
@@ -88,23 +88,6 @@ class CommandLineCompletionTestCase(Test
     @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
     @skipIfFreeBSD  # timing out on the FreeBSD buildbot
     @no_debug_info_test
-    def test_watchpoint_set_variable_dash_w(self):
-        """Test that 'watchpoint set variable -w' completes to 'watchpoint set variable -w '."""
-        self.complete_from_to(
-            'watchpoint set variable -w',
-            'watchpoint set variable -w ')
-
-    @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
-    @skipIfFreeBSD  # timing out on the FreeBSD buildbot
-    @no_debug_info_test
-    def test_watchpoint_set_variable_dash_w_space(self):
-        """Test that 'watchpoint set variable -w ' completes to ['read', 'write', 'read_write']."""
-        self.complete_from_to('watchpoint set variable -w ',
-                              ['read', 'write', 'read_write'])
-
-    @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
-    @skipIfFreeBSD  # timing out on the FreeBSD buildbot
-    @no_debug_info_test
     def test_watchpoint_set_ex(self):
         """Test that 'watchpoint set ex' completes to 'watchpoint set expression '."""
         self.complete_from_to(
@@ -121,15 +104,6 @@ class CommandLineCompletionTestCase(Test
     @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
     @skipIfFreeBSD  # timing out on the FreeBSD buildbot
     @no_debug_info_test
-    def test_watchpoint_set_variable_dash_w_read_underbar(self):
-        """Test that 'watchpoint set variable -w read_' completes to 'watchpoint set variable -w read_write'."""
-        self.complete_from_to(
-            'watchpoint set variable -w read_',
-            'watchpoint set variable -w read_write')
-
-    @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
-    @skipIfFreeBSD  # timing out on the FreeBSD buildbot
-    @no_debug_info_test
     def test_help_fi(self):
         """Test that 'help fi' completes to ['file', 'finish']."""
         self.complete_from_to(
@@ -296,6 +270,27 @@ class CommandLineCompletionTestCase(Test
         """Test that 'target va' completes to 'target variable '."""
         self.complete_from_to('target va', 'target variable ')
 
+    def test_command_argument_completion(self):
+        """Test completion of command arguments"""
+        self.complete_from_to("watchpoint set variable -", ["-w", "-s"])
+        self.complete_from_to('watchpoint set variable -w', 'watchpoint set variable -w ')
+        self.complete_from_to("watchpoint set variable --", ["--watch", "--size"])
+        self.complete_from_to("watchpoint set variable --w", "watchpoint set variable --watch")
+        self.complete_from_to('watchpoint set variable -w ', ['read', 'write', 'read_write'])
+        self.complete_from_to("watchpoint set variable --watch ", ["read", "write", "read_write"])
+        self.complete_from_to("watchpoint set variable --watch w", "watchpoint set variable --watch write")
+        self.complete_from_to('watchpoint set variable -w read_', 'watchpoint set variable -w read_write')
+        # Now try the same thing with a variable name (non-option argument) to
+        # test that getopts arg reshuffling doesn't confuse us.
+        self.complete_from_to("watchpoint set variable foo -", ["-w", "-s"])
+        self.complete_from_to('watchpoint set variable foo -w', 'watchpoint set variable foo -w ')
+        self.complete_from_to("watchpoint set variable foo --", ["--watch", "--size"])
+        self.complete_from_to("watchpoint set variable foo --w", "watchpoint set variable foo --watch")
+        self.complete_from_to('watchpoint set variable foo -w ', ['read', 'write', 'read_write'])
+        self.complete_from_to("watchpoint set variable foo --watch ", ["read", "write", "read_write"])
+        self.complete_from_to("watchpoint set variable foo --watch w", "watchpoint set variable foo --watch write")
+        self.complete_from_to('watchpoint set variable foo -w read_', 'watchpoint set variable foo -w read_write')
+
     @expectedFailureAll(hostoslist=["windows"], bugnumber="llvm.org/pr24679")
     def test_symbol_name(self):
         self.build()

Modified: lldb/trunk/source/Commands/CommandObjectSettings.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectSettings.cpp?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectSettings.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectSettings.cpp Fri Mar  9 02:39:40 2018
@@ -145,7 +145,7 @@ insert-before or insert-after.");
     const size_t argc = input.GetArgumentCount();
     const char *arg = nullptr;
     int setting_var_idx;
-    for (setting_var_idx = 1; setting_var_idx < static_cast<int>(argc);
+    for (setting_var_idx = 0; setting_var_idx < static_cast<int>(argc);
          ++setting_var_idx) {
       arg = input.GetArgumentAtIndex(setting_var_idx);
       if (arg && arg[0] != '-')

Modified: lldb/trunk/source/Interpreter/Args.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/Args.cpp (original)
+++ lldb/trunk/source/Interpreter/Args.cpp Fri Mar  9 02:39:40 2018
@@ -13,11 +13,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "lldb/DataFormatters/FormatManager.h"
-#include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/Args.h"
-#include "lldb/Interpreter/CommandInterpreter.h"
-#include "lldb/Interpreter/CommandReturnObject.h"
-#include "lldb/Interpreter/Options.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
@@ -269,32 +265,6 @@ void Args::SetCommandString(llvm::String
   m_argv.push_back(nullptr);
 }
 
-void Args::UpdateArgsAfterOptionParsing() {
-  assert(!m_argv.empty());
-  assert(m_argv.back() == nullptr);
-
-  // Now m_argv might be out of date with m_entries, so we need to fix that.
-  // This happens because getopt_long_only may permute the order of the
-  // arguments in argv, so we need to re-order the quotes and the refs array
-  // to match.
-  for (size_t i = 0; i < m_argv.size() - 1; ++i) {
-    const char *argv = m_argv[i];
-    auto pos =
-        std::find_if(m_entries.begin() + i, m_entries.end(),
-                     [argv](const ArgEntry &D) { return D.c_str() == argv; });
-    assert(pos != m_entries.end());
-    size_t distance = std::distance(m_entries.begin(), pos);
-    if (i == distance)
-      continue;
-
-    assert(distance > i);
-
-    std::swap(m_entries[i], m_entries[distance]);
-    assert(m_entries[i].ref.data() == m_argv[i]);
-  }
-  m_entries.resize(m_argv.size() - 1);
-}
-
 size_t Args::GetArgumentCount() const { return m_entries.size(); }
 
 const char *Args::GetArgumentAtIndex(size_t idx) const {
@@ -425,125 +395,6 @@ void Args::SetArguments(const char **arg
   SetArguments(ArgvToArgc(argv), argv);
 }
 
-Status Args::ParseOptions(Options &options, ExecutionContext *execution_context,
-                          PlatformSP platform_sp, bool require_validation) {
-  StreamString sstr;
-  Status error;
-  Option *long_options = options.GetLongOptions();
-  if (long_options == nullptr) {
-    error.SetErrorStringWithFormat("invalid long options");
-    return error;
-  }
-
-  for (int i = 0; long_options[i].definition != nullptr; ++i) {
-    if (long_options[i].flag == nullptr) {
-      if (isprint8(long_options[i].val)) {
-        sstr << (char)long_options[i].val;
-        switch (long_options[i].definition->option_has_arg) {
-        default:
-        case OptionParser::eNoArgument:
-          break;
-        case OptionParser::eRequiredArgument:
-          sstr << ':';
-          break;
-        case OptionParser::eOptionalArgument:
-          sstr << "::";
-          break;
-        }
-      }
-    }
-  }
-  std::unique_lock<std::mutex> lock;
-  OptionParser::Prepare(lock);
-  int val;
-  while (1) {
-    int long_options_index = -1;
-    val = OptionParser::Parse(GetArgumentCount(), GetArgumentVector(),
-                              sstr.GetString(), long_options,
-                              &long_options_index);
-    if (val == -1)
-      break;
-
-    // Did we get an error?
-    if (val == '?') {
-      error.SetErrorStringWithFormat("unknown or ambiguous option");
-      break;
-    }
-    // The option auto-set itself
-    if (val == 0)
-      continue;
-
-    ((Options *)&options)->OptionSeen(val);
-
-    // Lookup the long option index
-    if (long_options_index == -1) {
-      for (int i = 0; long_options[i].definition || long_options[i].flag ||
-                      long_options[i].val;
-           ++i) {
-        if (long_options[i].val == val) {
-          long_options_index = i;
-          break;
-        }
-      }
-    }
-    // Call the callback with the option
-    if (long_options_index >= 0 &&
-        long_options[long_options_index].definition) {
-      const OptionDefinition *def = long_options[long_options_index].definition;
-
-      if (!platform_sp) {
-        // User did not pass in an explicit platform.  Try to grab
-        // from the execution context.
-        TargetSP target_sp =
-            execution_context ? execution_context->GetTargetSP() : TargetSP();
-        platform_sp = target_sp ? target_sp->GetPlatform() : PlatformSP();
-      }
-      OptionValidator *validator = def->validator;
-
-      if (!platform_sp && require_validation) {
-        // Caller requires validation but we cannot validate as we
-        // don't have the mandatory platform against which to
-        // validate.
-        error.SetErrorString("cannot validate options: "
-                             "no platform available");
-        return error;
-      }
-
-      bool validation_failed = false;
-      if (platform_sp) {
-        // Ensure we have an execution context, empty or not.
-        ExecutionContext dummy_context;
-        ExecutionContext *exe_ctx_p =
-            execution_context ? execution_context : &dummy_context;
-        if (validator && !validator->IsValid(*platform_sp, *exe_ctx_p)) {
-          validation_failed = true;
-          error.SetErrorStringWithFormat("Option \"%s\" invalid.  %s",
-                                         def->long_option,
-                                         def->validator->LongConditionString());
-        }
-      }
-
-      // As long as validation didn't fail, we set the option value.
-      if (!validation_failed)
-        error = options.SetOptionValue(
-            long_options_index,
-            (def->option_has_arg == OptionParser::eNoArgument)
-                ? nullptr
-                : OptionParser::GetOptionArgument(),
-            execution_context);
-    } else {
-      error.SetErrorStringWithFormat("invalid option with value '%i'", val);
-    }
-    if (error.Fail())
-      break;
-  }
-
-  // Update our ARGV now that get options has consumed all the options
-  m_argv.erase(m_argv.begin(), m_argv.begin() + OptionParser::GetOptionIndex());
-  UpdateArgsAfterOptionParsing();
-  return error;
-}
-
 void Args::Clear() {
   m_entries.clear();
   m_argv.clear();
@@ -900,384 +751,6 @@ uint32_t Args::StringToGenericRegister(l
   return result;
 }
 
-size_t Args::FindArgumentIndexForOption(Option *long_options,
-                                        int long_options_index) const {
-  char short_buffer[3];
-  char long_buffer[255];
-  ::snprintf(short_buffer, sizeof(short_buffer), "-%c",
-             long_options[long_options_index].val);
-  ::snprintf(long_buffer, sizeof(long_buffer), "--%s",
-             long_options[long_options_index].definition->long_option);
-
-  for (auto entry : llvm::enumerate(m_entries)) {
-    if (entry.value().ref.startswith(short_buffer) ||
-        entry.value().ref.startswith(long_buffer))
-      return entry.index();
-  }
-
-  return size_t(-1);
-}
-
-bool Args::IsPositionalArgument(const char *arg) {
-  if (arg == nullptr)
-    return false;
-
-  bool is_positional = true;
-  const char *cptr = arg;
-
-  if (cptr[0] == '%') {
-    ++cptr;
-    while (isdigit(cptr[0]))
-      ++cptr;
-    if (cptr[0] != '\0')
-      is_positional = false;
-  } else
-    is_positional = false;
-
-  return is_positional;
-}
-
-std::string Args::ParseAliasOptions(Options &options,
-                                    CommandReturnObject &result,
-                                    OptionArgVector *option_arg_vector,
-                                    llvm::StringRef raw_input_string) {
-  std::string result_string(raw_input_string);
-  StreamString sstr;
-  int i;
-  Option *long_options = options.GetLongOptions();
-
-  if (long_options == nullptr) {
-    result.AppendError("invalid long options");
-    result.SetStatus(eReturnStatusFailed);
-    return result_string;
-  }
-
-  for (i = 0; long_options[i].definition != nullptr; ++i) {
-    if (long_options[i].flag == nullptr) {
-      sstr << (char)long_options[i].val;
-      switch (long_options[i].definition->option_has_arg) {
-      default:
-      case OptionParser::eNoArgument:
-        break;
-      case OptionParser::eRequiredArgument:
-        sstr << ":";
-        break;
-      case OptionParser::eOptionalArgument:
-        sstr << "::";
-        break;
-      }
-    }
-  }
-
-  std::unique_lock<std::mutex> lock;
-  OptionParser::Prepare(lock);
-  result.SetStatus(eReturnStatusSuccessFinishNoResult);
-  int val;
-  while (1) {
-    int long_options_index = -1;
-    val = OptionParser::Parse(GetArgumentCount(), GetArgumentVector(),
-                              sstr.GetString(), long_options,
-                              &long_options_index);
-
-    if (val == -1)
-      break;
-
-    if (val == '?') {
-      result.AppendError("unknown or ambiguous option");
-      result.SetStatus(eReturnStatusFailed);
-      break;
-    }
-
-    if (val == 0)
-      continue;
-
-    options.OptionSeen(val);
-
-    // Look up the long option index
-    if (long_options_index == -1) {
-      for (int j = 0; long_options[j].definition || long_options[j].flag ||
-                      long_options[j].val;
-           ++j) {
-        if (long_options[j].val == val) {
-          long_options_index = j;
-          break;
-        }
-      }
-    }
-
-    // See if the option takes an argument, and see if one was supplied.
-    if (long_options_index == -1) {
-      result.AppendErrorWithFormat("Invalid option with value '%c'.\n", val);
-      result.SetStatus(eReturnStatusFailed);
-      return result_string;
-    }
-
-    StreamString option_str;
-    option_str.Printf("-%c", val);
-    const OptionDefinition *def = long_options[long_options_index].definition;
-    int has_arg =
-        (def == nullptr) ? OptionParser::eNoArgument : def->option_has_arg;
-
-    const char *option_arg = nullptr;
-    switch (has_arg) {
-    case OptionParser::eRequiredArgument:
-      if (OptionParser::GetOptionArgument() == nullptr) {
-        result.AppendErrorWithFormat(
-            "Option '%s' is missing argument specifier.\n",
-            option_str.GetData());
-        result.SetStatus(eReturnStatusFailed);
-        return result_string;
-      }
-      LLVM_FALLTHROUGH;
-    case OptionParser::eOptionalArgument:
-      option_arg = OptionParser::GetOptionArgument();
-      LLVM_FALLTHROUGH;
-    case OptionParser::eNoArgument:
-      break;
-    default:
-      result.AppendErrorWithFormat("error with options table; invalid value "
-                                   "in has_arg field for option '%c'.\n",
-                                   val);
-      result.SetStatus(eReturnStatusFailed);
-      return result_string;
-    }
-    if (!option_arg)
-      option_arg = "<no-argument>";
-    option_arg_vector->emplace_back(option_str.GetString(), has_arg,
-                                    option_arg);
-
-    // Find option in the argument list; also see if it was supposed to take
-    // an argument and if one was supplied.  Remove option (and argument, if
-    // given) from the argument list.  Also remove them from the
-    // raw_input_string, if one was passed in.
-    size_t idx = FindArgumentIndexForOption(long_options, long_options_index);
-    if (idx == size_t(-1))
-      continue;
-
-    if (!result_string.empty()) {
-      auto tmp_arg = m_entries[idx].ref;
-      size_t pos = result_string.find(tmp_arg);
-      if (pos != std::string::npos)
-        result_string.erase(pos, tmp_arg.size());
-    }
-    ReplaceArgumentAtIndex(idx, llvm::StringRef());
-    if ((long_options[long_options_index].definition->option_has_arg !=
-         OptionParser::eNoArgument) &&
-        (OptionParser::GetOptionArgument() != nullptr) &&
-        (idx + 1 < GetArgumentCount()) &&
-        (m_entries[idx + 1].ref == OptionParser::GetOptionArgument())) {
-      if (result_string.size() > 0) {
-        auto tmp_arg = m_entries[idx + 1].ref;
-        size_t pos = result_string.find(tmp_arg);
-        if (pos != std::string::npos)
-          result_string.erase(pos, tmp_arg.size());
-      }
-      ReplaceArgumentAtIndex(idx + 1, llvm::StringRef());
-    }
-  }
-  return result_string;
-}
-
-void Args::ParseArgsForCompletion(Options &options,
-                                  OptionElementVector &option_element_vector,
-                                  uint32_t cursor_index) {
-  StreamString sstr;
-  Option *long_options = options.GetLongOptions();
-  option_element_vector.clear();
-
-  if (long_options == nullptr) {
-    return;
-  }
-
-  // Leading : tells getopt to return a : for a missing option argument AND
-  // to suppress error messages.
-
-  sstr << ":";
-  for (int i = 0; long_options[i].definition != nullptr; ++i) {
-    if (long_options[i].flag == nullptr) {
-      sstr << (char)long_options[i].val;
-      switch (long_options[i].definition->option_has_arg) {
-      default:
-      case OptionParser::eNoArgument:
-        break;
-      case OptionParser::eRequiredArgument:
-        sstr << ":";
-        break;
-      case OptionParser::eOptionalArgument:
-        sstr << "::";
-        break;
-      }
-    }
-  }
-
-  std::unique_lock<std::mutex> lock;
-  OptionParser::Prepare(lock);
-  OptionParser::EnableError(false);
-
-  int val;
-  auto opt_defs = options.GetDefinitions();
-
-  // Fooey... OptionParser::Parse permutes the GetArgumentVector to move the
-  // options to the front. So we have to build another Arg and pass that to
-  // OptionParser::Parse so it doesn't change the one we have.
-
-  std::vector<char *> dummy_vec = m_argv;
-
-  bool failed_once = false;
-  uint32_t dash_dash_pos = -1;
-
-  while (1) {
-    bool missing_argument = false;
-    int long_options_index = -1;
-
-    val = OptionParser::Parse(dummy_vec.size() - 1, &dummy_vec[0],
-                              sstr.GetString(), long_options,
-                              &long_options_index);
-
-    if (val == -1) {
-      // When we're completing a "--" which is the last option on line,
-      if (failed_once)
-        break;
-
-      failed_once = true;
-
-      // If this is a bare  "--" we mark it as such so we can complete it
-      // successfully later.
-      // Handling the "--" is a little tricky, since that may mean end of
-      // options or arguments, or the
-      // user might want to complete options by long name.  I make this work by
-      // checking whether the
-      // cursor is in the "--" argument, and if so I assume we're completing the
-      // long option, otherwise
-      // I let it pass to OptionParser::Parse which will terminate the option
-      // parsing.
-      // Note, in either case we continue parsing the line so we can figure out
-      // what other options
-      // were passed.  This will be useful when we come to restricting
-      // completions based on what other
-      // options we've seen on the line.
-
-      if (static_cast<size_t>(OptionParser::GetOptionIndex()) <
-              dummy_vec.size() - 1 &&
-          (strcmp(dummy_vec[OptionParser::GetOptionIndex() - 1], "--") == 0)) {
-        dash_dash_pos = OptionParser::GetOptionIndex() - 1;
-        if (static_cast<size_t>(OptionParser::GetOptionIndex() - 1) ==
-            cursor_index) {
-          option_element_vector.push_back(
-              OptionArgElement(OptionArgElement::eBareDoubleDash,
-                               OptionParser::GetOptionIndex() - 1,
-                               OptionArgElement::eBareDoubleDash));
-          continue;
-        } else
-          break;
-      } else
-        break;
-    } else if (val == '?') {
-      option_element_vector.push_back(
-          OptionArgElement(OptionArgElement::eUnrecognizedArg,
-                           OptionParser::GetOptionIndex() - 1,
-                           OptionArgElement::eUnrecognizedArg));
-      continue;
-    } else if (val == 0) {
-      continue;
-    } else if (val == ':') {
-      // This is a missing argument.
-      val = OptionParser::GetOptionErrorCause();
-      missing_argument = true;
-    }
-
-    ((Options *)&options)->OptionSeen(val);
-
-    // Look up the long option index
-    if (long_options_index == -1) {
-      for (int j = 0; long_options[j].definition || long_options[j].flag ||
-                      long_options[j].val;
-           ++j) {
-        if (long_options[j].val == val) {
-          long_options_index = j;
-          break;
-        }
-      }
-    }
-
-    // See if the option takes an argument, and see if one was supplied.
-    if (long_options_index >= 0) {
-      int opt_defs_index = -1;
-      for (size_t i = 0; i < opt_defs.size(); i++) {
-        if (opt_defs[i].short_option != val)
-          continue;
-        opt_defs_index = i;
-        break;
-      }
-
-      const OptionDefinition *def = long_options[long_options_index].definition;
-      int has_arg =
-          (def == nullptr) ? OptionParser::eNoArgument : def->option_has_arg;
-      switch (has_arg) {
-      case OptionParser::eNoArgument:
-        option_element_vector.push_back(OptionArgElement(
-            opt_defs_index, OptionParser::GetOptionIndex() - 1, 0));
-        break;
-      case OptionParser::eRequiredArgument:
-        if (OptionParser::GetOptionArgument() != nullptr) {
-          int arg_index;
-          if (missing_argument)
-            arg_index = -1;
-          else
-            arg_index = OptionParser::GetOptionIndex() - 1;
-
-          option_element_vector.push_back(OptionArgElement(
-              opt_defs_index, OptionParser::GetOptionIndex() - 2, arg_index));
-        } else {
-          option_element_vector.push_back(OptionArgElement(
-              opt_defs_index, OptionParser::GetOptionIndex() - 1, -1));
-        }
-        break;
-      case OptionParser::eOptionalArgument:
-        if (OptionParser::GetOptionArgument() != nullptr) {
-          option_element_vector.push_back(OptionArgElement(
-              opt_defs_index, OptionParser::GetOptionIndex() - 2,
-              OptionParser::GetOptionIndex() - 1));
-        } else {
-          option_element_vector.push_back(OptionArgElement(
-              opt_defs_index, OptionParser::GetOptionIndex() - 2,
-              OptionParser::GetOptionIndex() - 1));
-        }
-        break;
-      default:
-        // The options table is messed up.  Here we'll just continue
-        option_element_vector.push_back(
-            OptionArgElement(OptionArgElement::eUnrecognizedArg,
-                             OptionParser::GetOptionIndex() - 1,
-                             OptionArgElement::eUnrecognizedArg));
-        break;
-      }
-    } else {
-      option_element_vector.push_back(
-          OptionArgElement(OptionArgElement::eUnrecognizedArg,
-                           OptionParser::GetOptionIndex() - 1,
-                           OptionArgElement::eUnrecognizedArg));
-    }
-  }
-
-  // Finally we have to handle the case where the cursor index points at a
-  // single "-".  We want to mark that in
-  // the option_element_vector, but only if it is not after the "--".  But it
-  // turns out that OptionParser::Parse just ignores
-  // an isolated "-".  So we have to look it up by hand here.  We only care if
-  // it is AT the cursor position.
-  // Note, a single quoted dash is not the same as a single dash...
-
-  const ArgEntry &cursor = m_entries[cursor_index];
-  if ((static_cast<int32_t>(dash_dash_pos) == -1 ||
-       cursor_index < dash_dash_pos) &&
-      cursor.quote == '\0' && cursor.ref == "-") {
-    option_element_vector.push_back(
-        OptionArgElement(OptionArgElement::eBareDash, cursor_index,
-                         OptionArgElement::eBareDash));
-  }
-}
-
 void Args::EncodeEscapeSequences(const char *src, std::string &dst) {
   dst.clear();
   if (src) {

Modified: lldb/trunk/source/Interpreter/CommandAlias.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandAlias.cpp?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/CommandAlias.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandAlias.cpp Fri Mar  9 02:39:40 2018
@@ -40,12 +40,17 @@ static bool ProcessAliasOptionsArgs(lldb
     ExecutionContext exe_ctx =
         cmd_obj_sp->GetCommandInterpreter().GetExecutionContext();
     options->NotifyOptionParsingStarting(&exe_ctx);
-    args.Unshift(llvm::StringRef("dummy_arg"));
-    options_string = args.ParseAliasOptions(*options, result, option_arg_vector,
-                                            options_args);
-    args.Shift();
-    if (result.Succeeded())
-      options->VerifyPartialOptions(result);
+
+    llvm::Expected<Args> args_or =
+        options->ParseAlias(args, option_arg_vector, options_string);
+    if (!args_or) {
+      result.AppendError(toString(args_or.takeError()));
+      result.AppendError("Unable to create requested alias.\n");
+      result.SetStatus(eReturnStatusFailed);
+      return false;
+    }
+    args = std::move(*args_or);
+    options->VerifyPartialOptions(result);
     if (!result.Succeeded() &&
         result.GetStatus() != lldb::eReturnStatusStarted) {
       result.AppendError("Unable to create requested alias.\n");

Modified: lldb/trunk/source/Interpreter/CommandObject.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandObject.cpp?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/CommandObject.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandObject.cpp Fri Mar  9 02:39:40 2018
@@ -104,20 +104,16 @@ bool CommandObject::ParseOptions(Args &a
     auto exe_ctx = GetCommandInterpreter().GetExecutionContext();
     options->NotifyOptionParsingStarting(&exe_ctx);
 
-    // ParseOptions calls getopt_long_only, which always skips the zero'th item
-    // in the array and starts at position 1,
-    // so we need to push a dummy value into position zero.
-    args.Unshift(llvm::StringRef("dummy_string"));
     const bool require_validation = true;
-    error = args.ParseOptions(*options, &exe_ctx,
-                              GetCommandInterpreter().GetPlatform(true),
-                              require_validation);
+    llvm::Expected<Args> args_or = options->Parse(
+        args, &exe_ctx, GetCommandInterpreter().GetPlatform(true),
+        require_validation);
 
-    // The "dummy_string" will have already been removed by ParseOptions,
-    // so no need to remove it.
-
-    if (error.Success())
+    if (args_or) {
+      args = std::move(*args_or);
       error = options->NotifyOptionParsingFinished(&exe_ctx);
+    } else
+      error = args_or.takeError();
 
     if (error.Success()) {
       if (options->VerifyOptions(result))
@@ -285,20 +281,7 @@ int CommandObject::HandleCompletion(Args
     OptionElementVector opt_element_vector;
 
     if (cur_options != nullptr) {
-      // Re-insert the dummy command name string which will have been
-      // stripped off:
-      input.Unshift(llvm::StringRef("dummy-string"));
-      cursor_index++;
-
-      // I stick an element on the end of the input, because if the last element
-      // is option that requires an argument, getopt_long_only will freak out.
-
-      input.AppendArgument(llvm::StringRef("<FAKE-VALUE>"));
-
-      input.ParseArgsForCompletion(*cur_options, opt_element_vector,
-                                   cursor_index);
-
-      input.DeleteArgumentAtIndex(input.GetArgumentCount() - 1);
+      opt_element_vector = cur_options->ParseForCompletion(input, cursor_index);
 
       bool handled_by_options;
       handled_by_options = cur_options->HandleOptionCompletion(

Modified: lldb/trunk/source/Interpreter/Options.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Options.cpp?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/Options.cpp (original)
+++ lldb/trunk/source/Interpreter/Options.cpp Fri Mar  9 02:39:40 2018
@@ -951,3 +951,527 @@ OptionGroupOptions::OptionParsingFinishe
   }
   return error;
 }
+
+// OptionParser permutes the arguments while processing them, so we create a
+// temporary array holding to avoid modification of the input arguments. The
+// options themselves are never modified, but the API expects a char * anyway,
+// hence the const_cast.
+static std::vector<char *> GetArgvForParsing(const Args &args) {
+  std::vector<char *> result;
+  // OptionParser always skips the first argument as it is based on getopt().
+  result.push_back(const_cast<char *>("<FAKE-ARG0>"));
+  for (const Args::ArgEntry &entry : args)
+    result.push_back(const_cast<char *>(entry.c_str()));
+  return result;
+}
+
+// Given a permuted argument, find it's position in the original Args vector.
+static Args::const_iterator FindOriginalIter(const char *arg,
+                                             const Args &original) {
+  return llvm::find_if(
+      original, [arg](const Args::ArgEntry &D) { return D.c_str() == arg; });
+}
+
+// Given a permuted argument, find it's index in the original Args vector.
+static size_t FindOriginalIndex(const char *arg, const Args &original) {
+  return std::distance(original.begin(), FindOriginalIter(arg, original));
+}
+
+// Construct a new Args object, consisting of the entries from the original
+// arguments, but in the permuted order.
+static Args ReconstituteArgsAfterParsing(llvm::ArrayRef<char *> parsed,
+                                         const Args &original) {
+  Args result;
+  for (const char *arg : parsed) {
+    auto pos = FindOriginalIter(arg, original);
+    assert(pos != original.end());
+    result.AppendArgument(pos->ref, pos->quote);
+  }
+  return result;
+}
+
+static size_t FindArgumentIndexForOption(const Args &args,
+                                         const Option &long_option) {
+  std::string short_opt = llvm::formatv("-{0}", char(long_option.val)).str();
+  std::string long_opt =
+      llvm::formatv("--{0}", long_option.definition->long_option);
+  for (const auto &entry : llvm::enumerate(args)) {
+    if (entry.value().ref.startswith(short_opt) ||
+        entry.value().ref.startswith(long_opt))
+      return entry.index();
+  }
+
+  return size_t(-1);
+}
+
+llvm::Expected<Args> Options::ParseAlias(const Args &args,
+                                         OptionArgVector *option_arg_vector,
+                                         std::string &input_line) {
+  StreamString sstr;
+  int i;
+  Option *long_options = GetLongOptions();
+
+  if (long_options == nullptr) {
+    return llvm::make_error<llvm::StringError>("Invalid long options",
+                                               llvm::inconvertibleErrorCode());
+  }
+
+  for (i = 0; long_options[i].definition != nullptr; ++i) {
+    if (long_options[i].flag == nullptr) {
+      sstr << (char)long_options[i].val;
+      switch (long_options[i].definition->option_has_arg) {
+      default:
+      case OptionParser::eNoArgument:
+        break;
+      case OptionParser::eRequiredArgument:
+        sstr << ":";
+        break;
+      case OptionParser::eOptionalArgument:
+        sstr << "::";
+        break;
+      }
+    }
+  }
+
+  Args args_copy = args;
+  std::vector<char *> argv = GetArgvForParsing(args);
+
+  std::unique_lock<std::mutex> lock;
+  OptionParser::Prepare(lock);
+  int val;
+  while (1) {
+    int long_options_index = -1;
+    val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
+                              long_options, &long_options_index);
+
+    if (val == -1)
+      break;
+
+    if (val == '?') {
+      return llvm::make_error<llvm::StringError>(
+          "Unknown or ambiguous option", llvm::inconvertibleErrorCode());
+    }
+
+    if (val == 0)
+      continue;
+
+    OptionSeen(val);
+
+    // Look up the long option index
+    if (long_options_index == -1) {
+      for (int j = 0; long_options[j].definition || long_options[j].flag ||
+                      long_options[j].val;
+           ++j) {
+        if (long_options[j].val == val) {
+          long_options_index = j;
+          break;
+        }
+      }
+    }
+
+    // See if the option takes an argument, and see if one was supplied.
+    if (long_options_index == -1) {
+      return llvm::make_error<llvm::StringError>(
+          llvm::formatv("Invalid option with value '{0}'.", char(val)).str(),
+          llvm::inconvertibleErrorCode());
+    }
+
+    StreamString option_str;
+    option_str.Printf("-%c", val);
+    const OptionDefinition *def = long_options[long_options_index].definition;
+    int has_arg =
+        (def == nullptr) ? OptionParser::eNoArgument : def->option_has_arg;
+
+    const char *option_arg = nullptr;
+    switch (has_arg) {
+    case OptionParser::eRequiredArgument:
+      if (OptionParser::GetOptionArgument() == nullptr) {
+        return llvm::make_error<llvm::StringError>(
+            llvm::formatv("Option '{0}' is missing argument specifier.",
+                          option_str.GetString())
+                .str(),
+            llvm::inconvertibleErrorCode());
+      }
+      LLVM_FALLTHROUGH;
+    case OptionParser::eOptionalArgument:
+      option_arg = OptionParser::GetOptionArgument();
+      LLVM_FALLTHROUGH;
+    case OptionParser::eNoArgument:
+      break;
+    default:
+      return llvm::make_error<llvm::StringError>(
+          llvm::formatv("error with options table; invalid value in has_arg "
+                        "field for option '{0}'.",
+                        char(val))
+              .str(),
+          llvm::inconvertibleErrorCode());
+    }
+    if (!option_arg)
+      option_arg = "<no-argument>";
+    option_arg_vector->emplace_back(option_str.GetString(), has_arg,
+                                    option_arg);
+
+    // Find option in the argument list; also see if it was supposed to take
+    // an argument and if one was supplied.  Remove option (and argument, if
+    // given) from the argument list.  Also remove them from the
+    // raw_input_string, if one was passed in.
+    size_t idx =
+        FindArgumentIndexForOption(args_copy, long_options[long_options_index]);
+    if (idx == size_t(-1))
+      continue;
+
+    if (!input_line.empty()) {
+      auto tmp_arg = args_copy[idx].ref;
+      size_t pos = input_line.find(tmp_arg);
+      if (pos != std::string::npos)
+        input_line.erase(pos, tmp_arg.size());
+    }
+    args_copy.DeleteArgumentAtIndex(idx);
+    if ((long_options[long_options_index].definition->option_has_arg !=
+         OptionParser::eNoArgument) &&
+        (OptionParser::GetOptionArgument() != nullptr) &&
+        (idx < args_copy.GetArgumentCount()) &&
+        (args_copy[idx].ref == OptionParser::GetOptionArgument())) {
+      if (input_line.size() > 0) {
+        auto tmp_arg = args_copy[idx].ref;
+        size_t pos = input_line.find(tmp_arg);
+        if (pos != std::string::npos)
+          input_line.erase(pos, tmp_arg.size());
+      }
+      args_copy.DeleteArgumentAtIndex(idx);
+    }
+  }
+
+  return std::move(args_copy);
+}
+
+OptionElementVector Options::ParseForCompletion(const Args &args,
+                                                uint32_t cursor_index) {
+  OptionElementVector option_element_vector;
+  StreamString sstr;
+  Option *long_options = GetLongOptions();
+  option_element_vector.clear();
+
+  if (long_options == nullptr)
+    return option_element_vector;
+
+  // Leading : tells getopt to return a : for a missing option argument AND
+  // to suppress error messages.
+
+  sstr << ":";
+  for (int i = 0; long_options[i].definition != nullptr; ++i) {
+    if (long_options[i].flag == nullptr) {
+      sstr << (char)long_options[i].val;
+      switch (long_options[i].definition->option_has_arg) {
+      default:
+      case OptionParser::eNoArgument:
+        break;
+      case OptionParser::eRequiredArgument:
+        sstr << ":";
+        break;
+      case OptionParser::eOptionalArgument:
+        sstr << "::";
+        break;
+      }
+    }
+  }
+
+  std::unique_lock<std::mutex> lock;
+  OptionParser::Prepare(lock);
+  OptionParser::EnableError(false);
+
+  int val;
+  auto opt_defs = GetDefinitions();
+
+  std::vector<char *> dummy_vec = GetArgvForParsing(args);
+
+  // I stick an element on the end of the input, because if the last element
+  // is option that requires an argument, getopt_long_only will freak out.
+  dummy_vec.push_back(const_cast<char *>("<FAKE-VALUE>"));
+
+  bool failed_once = false;
+  uint32_t dash_dash_pos = -1;
+
+  while (1) {
+    bool missing_argument = false;
+    int long_options_index = -1;
+
+    val = OptionParser::Parse(dummy_vec.size(), &dummy_vec[0], sstr.GetString(),
+                              long_options, &long_options_index);
+
+    if (val == -1) {
+      // When we're completing a "--" which is the last option on line,
+      if (failed_once)
+        break;
+
+      failed_once = true;
+
+      // If this is a bare  "--" we mark it as such so we can complete it
+      // successfully later.  Handling the "--" is a little tricky, since that
+      // may mean end of options or arguments, or the user might want to
+      // complete options by long name.  I make this work by checking whether
+      // the cursor is in the "--" argument, and if so I assume we're
+      // completing the long option, otherwise I let it pass to
+      // OptionParser::Parse which will terminate the option parsing.  Note, in
+      // either case we continue parsing the line so we can figure out what
+      // other options were passed.  This will be useful when we come to
+      // restricting completions based on what other options we've seen on the
+      // line.
+
+      if (static_cast<size_t>(OptionParser::GetOptionIndex()) <
+              dummy_vec.size() &&
+          (strcmp(dummy_vec[OptionParser::GetOptionIndex() - 1], "--") == 0)) {
+        dash_dash_pos = FindOriginalIndex(
+            dummy_vec[OptionParser::GetOptionIndex() - 1], args);
+        if (dash_dash_pos == cursor_index) {
+          option_element_vector.push_back(
+              OptionArgElement(OptionArgElement::eBareDoubleDash, dash_dash_pos,
+                               OptionArgElement::eBareDoubleDash));
+          continue;
+        } else
+          break;
+      } else
+        break;
+    } else if (val == '?') {
+      option_element_vector.push_back(OptionArgElement(
+          OptionArgElement::eUnrecognizedArg,
+          FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                            args),
+          OptionArgElement::eUnrecognizedArg));
+      continue;
+    } else if (val == 0) {
+      continue;
+    } else if (val == ':') {
+      // This is a missing argument.
+      val = OptionParser::GetOptionErrorCause();
+      missing_argument = true;
+    }
+
+    OptionSeen(val);
+
+    // Look up the long option index
+    if (long_options_index == -1) {
+      for (int j = 0; long_options[j].definition || long_options[j].flag ||
+                      long_options[j].val;
+           ++j) {
+        if (long_options[j].val == val) {
+          long_options_index = j;
+          break;
+        }
+      }
+    }
+
+    // See if the option takes an argument, and see if one was supplied.
+    if (long_options_index >= 0) {
+      int opt_defs_index = -1;
+      for (size_t i = 0; i < opt_defs.size(); i++) {
+        if (opt_defs[i].short_option != val)
+          continue;
+        opt_defs_index = i;
+        break;
+      }
+
+      const OptionDefinition *def = long_options[long_options_index].definition;
+      int has_arg =
+          (def == nullptr) ? OptionParser::eNoArgument : def->option_has_arg;
+      switch (has_arg) {
+      case OptionParser::eNoArgument:
+        option_element_vector.push_back(OptionArgElement(
+            opt_defs_index,
+            FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                              args),
+            0));
+        break;
+      case OptionParser::eRequiredArgument:
+        if (OptionParser::GetOptionArgument() != nullptr) {
+          int arg_index;
+          if (missing_argument)
+            arg_index = -1;
+          else
+            arg_index = OptionParser::GetOptionIndex() - 2;
+
+          option_element_vector.push_back(OptionArgElement(
+              opt_defs_index,
+              FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 2],
+                                args),
+              arg_index));
+        } else {
+          option_element_vector.push_back(OptionArgElement(
+              opt_defs_index,
+              FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                                args),
+              -1));
+        }
+        break;
+      case OptionParser::eOptionalArgument:
+        if (OptionParser::GetOptionArgument() != nullptr) {
+          option_element_vector.push_back(OptionArgElement(
+              opt_defs_index,
+              FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 2],
+                                args),
+              FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                                args)));
+        } else {
+          option_element_vector.push_back(OptionArgElement(
+              opt_defs_index,
+              FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 2],
+                                args),
+              FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                                args)));
+        }
+        break;
+      default:
+        // The options table is messed up.  Here we'll just continue
+        option_element_vector.push_back(OptionArgElement(
+            OptionArgElement::eUnrecognizedArg,
+            FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                              args),
+            OptionArgElement::eUnrecognizedArg));
+        break;
+      }
+    } else {
+      option_element_vector.push_back(OptionArgElement(
+          OptionArgElement::eUnrecognizedArg,
+          FindOriginalIndex(dummy_vec[OptionParser::GetOptionIndex() - 1],
+                            args),
+          OptionArgElement::eUnrecognizedArg));
+    }
+  }
+
+  // Finally we have to handle the case where the cursor index points at a
+  // single "-".  We want to mark that in
+  // the option_element_vector, but only if it is not after the "--".  But it
+  // turns out that OptionParser::Parse just ignores
+  // an isolated "-".  So we have to look it up by hand here.  We only care if
+  // it is AT the cursor position.
+  // Note, a single quoted dash is not the same as a single dash...
+
+  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 == "-") {
+    option_element_vector.push_back(
+        OptionArgElement(OptionArgElement::eBareDash, cursor_index,
+                         OptionArgElement::eBareDash));
+  }
+  return option_element_vector;
+}
+
+llvm::Expected<Args> Options::Parse(const Args &args,
+                                    ExecutionContext *execution_context,
+                                    lldb::PlatformSP platform_sp,
+                                    bool require_validation) {
+  StreamString sstr;
+  Status error;
+  Option *long_options = GetLongOptions();
+  if (long_options == nullptr) {
+    return llvm::make_error<llvm::StringError>("Invalid long options.",
+                                               llvm::inconvertibleErrorCode());
+  }
+
+  for (int i = 0; long_options[i].definition != nullptr; ++i) {
+    if (long_options[i].flag == nullptr) {
+      if (isprint8(long_options[i].val)) {
+        sstr << (char)long_options[i].val;
+        switch (long_options[i].definition->option_has_arg) {
+        default:
+        case OptionParser::eNoArgument:
+          break;
+        case OptionParser::eRequiredArgument:
+          sstr << ':';
+          break;
+        case OptionParser::eOptionalArgument:
+          sstr << "::";
+          break;
+        }
+      }
+    }
+  }
+  std::vector<char *> argv = GetArgvForParsing(args);
+  std::unique_lock<std::mutex> lock;
+  OptionParser::Prepare(lock);
+  int val;
+  while (1) {
+    int long_options_index = -1;
+    val = OptionParser::Parse(argv.size(), &*argv.begin(), sstr.GetString(),
+                              long_options, &long_options_index);
+    if (val == -1)
+      break;
+
+    // Did we get an error?
+    if (val == '?') {
+      error.SetErrorStringWithFormat("unknown or ambiguous option");
+      break;
+    }
+    // The option auto-set itself
+    if (val == 0)
+      continue;
+
+    OptionSeen(val);
+
+    // Lookup the long option index
+    if (long_options_index == -1) {
+      for (int i = 0; long_options[i].definition || long_options[i].flag ||
+                      long_options[i].val;
+           ++i) {
+        if (long_options[i].val == val) {
+          long_options_index = i;
+          break;
+        }
+      }
+    }
+    // Call the callback with the option
+    if (long_options_index >= 0 &&
+        long_options[long_options_index].definition) {
+      const OptionDefinition *def = long_options[long_options_index].definition;
+
+      if (!platform_sp) {
+        // User did not pass in an explicit platform.  Try to grab
+        // from the execution context.
+        TargetSP target_sp =
+            execution_context ? execution_context->GetTargetSP() : TargetSP();
+        platform_sp = target_sp ? target_sp->GetPlatform() : PlatformSP();
+      }
+      OptionValidator *validator = def->validator;
+
+      if (!platform_sp && require_validation) {
+        // Caller requires validation but we cannot validate as we
+        // don't have the mandatory platform against which to
+        // validate.
+        return llvm::make_error<llvm::StringError>(
+            "cannot validate options: no platform available",
+            llvm::inconvertibleErrorCode());
+      }
+
+      bool validation_failed = false;
+      if (platform_sp) {
+        // Ensure we have an execution context, empty or not.
+        ExecutionContext dummy_context;
+        ExecutionContext *exe_ctx_p =
+            execution_context ? execution_context : &dummy_context;
+        if (validator && !validator->IsValid(*platform_sp, *exe_ctx_p)) {
+          validation_failed = true;
+          error.SetErrorStringWithFormat("Option \"%s\" invalid.  %s",
+                                         def->long_option,
+                                         def->validator->LongConditionString());
+        }
+      }
+
+      // As long as validation didn't fail, we set the option value.
+      if (!validation_failed)
+        error =
+            SetOptionValue(long_options_index,
+                           (def->option_has_arg == OptionParser::eNoArgument)
+                               ? nullptr
+                               : OptionParser::GetOptionArgument(),
+                           execution_context);
+    } else {
+      error.SetErrorStringWithFormat("invalid option with value '%i'", val);
+    }
+    if (error.Fail())
+      return error.ToError();
+  }
+
+  argv.erase(argv.begin(), argv.begin() + OptionParser::GetOptionIndex());
+  return ReconstituteArgsAfterParsing(argv, args);
+}

Modified: lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp?rev=327110&r1=327109&r2=327110&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp (original)
+++ lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp Fri Mar  9 02:39:40 2018
@@ -1013,6 +1013,7 @@ public:
 };
 
 EnableOptionsSP ParseAutoEnableOptions(Status &error, Debugger &debugger) {
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS);
   // We are abusing the options data model here so that we can parse
   // options without requiring the Debugger instance.
 
@@ -1051,15 +1052,16 @@ EnableOptionsSP ParseAutoEnableOptions(S
       args.Shift();
   }
 
-  // ParseOptions calls getopt_long_only, which always skips the zero'th item in
-  // the array and starts at position 1,
-  // so we need to push a dummy value into position zero.
-  args.Unshift(llvm::StringRef("dummy_string"));
   bool require_validation = false;
-  error = args.ParseOptions(*options_sp.get(), &exe_ctx, PlatformSP(),
-                            require_validation);
-  if (!error.Success())
+  llvm::Expected<Args> args_or =
+      options_sp->Parse(args, &exe_ctx, PlatformSP(), require_validation);
+  if (!args_or) {
+    LLDB_LOG_ERROR(
+        log, args_or.takeError(),
+        "Parsing plugin.structured-data.darwin-log.auto-enable-options value "
+        "failed: {0}");
     return EnableOptionsSP();
+  }
 
   if (!options_sp->VerifyOptions(result))
     return EnableOptionsSP();




More information about the lldb-commits mailing list