Index: lldb/trunk/include/lldb/Interpreter/Args.h =================================================================== --- lldb/trunk/include/lldb/Interpreter/Args.h +++ lldb/trunk/include/lldb/Interpreter/Args.h @@ -28,8 +28,6 @@ namespace lldb_private { -struct Option; - typedef std::vector> OptionArgVector; typedef std::shared_ptr OptionArgVectorSP; @@ -161,10 +159,10 @@ llvm::ArrayRef entries() const { return m_entries; } char GetArgumentQuoteCharAtIndex(size_t idx) const; - std::vector::const_iterator begin() const { - return m_entries.begin(); - } - std::vector::const_iterator end() const { return m_entries.end(); } + using const_iterator = std::vector::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 @@ 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 @@ char quote_char); private: - size_t FindArgumentIndexForOption(Option *long_options, - int long_options_index) const; - std::vector m_entries; std::vector m_argv; - - void UpdateArgsAfterOptionParsing(); }; } // namespace lldb_private Index: lldb/trunk/include/lldb/Interpreter/Options.h =================================================================== --- lldb/trunk/include/lldb/Interpreter/Options.h +++ lldb/trunk/include/lldb/Interpreter/Options.h @@ -25,6 +25,8 @@ namespace lldb_private { +struct Option; + static inline bool isprint8(int ch) { if (ch & 0xffffff00u) return false; @@ -36,10 +38,8 @@ /// @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 @@ /// *optstring, const struct option *longopts, int *longindex); /// \endcode /// -/// Example code: -/// \code -/// #include -/// #include -/// -/// 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 @@ // 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 Parse(const Args &args, + ExecutionContext *execution_context, + lldb::PlatformSP platform_sp, + bool require_validation); + + llvm::Expected 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); //------------------------------------------------------------------ Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py =================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py @@ -88,23 +88,6 @@ @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 @@ @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 @@ """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() Index: lldb/trunk/source/Commands/CommandObjectSettings.cpp =================================================================== --- lldb/trunk/source/Commands/CommandObjectSettings.cpp +++ lldb/trunk/source/Commands/CommandObjectSettings.cpp @@ -145,7 +145,7 @@ const size_t argc = input.GetArgumentCount(); const char *arg = nullptr; int setting_var_idx; - for (setting_var_idx = 1; setting_var_idx < static_cast(argc); + for (setting_var_idx = 0; setting_var_idx < static_cast(argc); ++setting_var_idx) { arg = input.GetArgumentAtIndex(setting_var_idx); if (arg && arg[0] != '-') Index: lldb/trunk/source/Interpreter/Args.cpp =================================================================== --- lldb/trunk/source/Interpreter/Args.cpp +++ lldb/trunk/source/Interpreter/Args.cpp @@ -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 @@ 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 @@ 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 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 @@ 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 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 = ""; - 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 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 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(OptionParser::GetOptionIndex()) < - dummy_vec.size() - 1 && - (strcmp(dummy_vec[OptionParser::GetOptionIndex() - 1], "--") == 0)) { - dash_dash_pos = OptionParser::GetOptionIndex() - 1; - if (static_cast(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(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) { Index: lldb/trunk/source/Interpreter/CommandAlias.cpp =================================================================== --- lldb/trunk/source/Interpreter/CommandAlias.cpp +++ lldb/trunk/source/Interpreter/CommandAlias.cpp @@ -40,12 +40,17 @@ 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_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"); Index: lldb/trunk/source/Interpreter/CommandObject.cpp =================================================================== --- lldb/trunk/source/Interpreter/CommandObject.cpp +++ lldb/trunk/source/Interpreter/CommandObject.cpp @@ -104,20 +104,16 @@ 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_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 @@ 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("")); - - 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( Index: lldb/trunk/source/Interpreter/Options.cpp =================================================================== --- lldb/trunk/source/Interpreter/Options.cpp +++ lldb/trunk/source/Interpreter/Options.cpp @@ -951,3 +951,527 @@ } 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 GetArgvForParsing(const Args &args) { + std::vector result; + // OptionParser always skips the first argument as it is based on getopt(). + result.push_back(const_cast("")); + for (const Args::ArgEntry &entry : args) + result.push_back(const_cast(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 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 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("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 argv = GetArgvForParsing(args); + + std::unique_lock 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( + "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::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::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::formatv("error with options table; invalid value in has_arg " + "field for option '{0}'.", + char(val)) + .str(), + llvm::inconvertibleErrorCode()); + } + if (!option_arg) + option_arg = ""; + 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 lock; + OptionParser::Prepare(lock); + OptionParser::EnableError(false); + + int val; + auto opt_defs = GetDefinitions(); + + std::vector 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("")); + + 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(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(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 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("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 argv = GetArgvForParsing(args); + std::unique_lock 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( + "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); +} Index: lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp =================================================================== --- lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp +++ lldb/trunk/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp @@ -1013,6 +1013,7 @@ }; 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 @@ 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_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();