teemperor created this revision. teemperor added a reviewer: jingham. Herald added a subscriber: mgorny.
A subset of the LLDB commands follows this command line interface style: <command name> [arguments] -- <string suffix> The parsing code for this interface has been so far been duplicated into the different command objects which makes it hard to maintain and reuse elsewhere. This patches improves the situation by adding a ArgsWithSuffix class that centralizes the parsing logic and allows easier testing. The different commands now just call this class to extract the arguments and the raw suffix from the provided user input. https://reviews.llvm.org/D49106 Files: include/lldb/Interpreter/CommandObject.h include/lldb/Utility/Args.h source/Commands/CommandObjectCommands.cpp source/Commands/CommandObjectExpression.cpp source/Commands/CommandObjectPlatform.cpp source/Commands/CommandObjectType.cpp source/Commands/CommandObjectWatchpoint.cpp source/Interpreter/CommandObject.cpp source/Interpreter/Options.cpp source/Utility/Args.cpp unittests/Utility/ArgsWithSuffixTest.cpp unittests/Utility/CMakeLists.txt
Index: unittests/Utility/CMakeLists.txt =================================================================== --- unittests/Utility/CMakeLists.txt +++ unittests/Utility/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(UtilityTests ArgsTest.cpp + ArgsWithSuffixTest.cpp ArchSpecTest.cpp CleanUpTest.cpp ConstStringTest.cpp Index: unittests/Utility/ArgsWithSuffixTest.cpp =================================================================== --- /dev/null +++ unittests/Utility/ArgsWithSuffixTest.cpp @@ -0,0 +1,183 @@ +//===-- ArgsWithSuffixTest.cpp ----------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "gtest/gtest.h" + +#include "lldb/Utility/Args.h" +#include "lldb/Utility/StringList.h" + +using namespace lldb_private; + +TEST(ArgsWithSuffixTest, EmptyInput) { + // An empty string is just an empty suffix without any arguments. + ArgsWithSuffix args(""); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetSuffix().c_str(), ""); +} + +TEST(ArgsWithSuffixTest, SingleWhitespaceInput) { + // Only whitespace is just a suffix. + ArgsWithSuffix args(" "); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetSuffix().c_str(), " "); +} + +TEST(ArgsWithSuffixTest, WhitespaceInput) { + // Only whitespace is just a suffix. + ArgsWithSuffix args(" "); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetSuffix().c_str(), " "); +} + +TEST(ArgsWithSuffixTest, ArgsButNoDelimiter) { + // This counts as a suffix because there is no -- at the end. + ArgsWithSuffix args("-foo bar"); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetSuffix().c_str(), "-foo bar"); +} + +TEST(ArgsWithSuffixTest, ArgsButNoLeadingDash) { + // No leading dash means we have no arguments. + ArgsWithSuffix args("foo bar --"); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetSuffix().c_str(), "foo bar --"); +} + +TEST(ArgsWithSuffixTest, QuotedSuffix) { + // We need to have a way to escape the -- to make it usable as an argument. + ArgsWithSuffix args("-foo \"--\" bar"); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetSuffix().c_str(), "-foo \"--\" bar"); +} + +TEST(ArgsWithSuffixTest, EmptySuffix) { + // An empty suffix with arguments. + ArgsWithSuffix args("-foo --"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo --"); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), ""); +} + +TEST(ArgsWithSuffixTest, EmptySuffixSingleWhitespace) { + // A single whitespace also countas as an empty suffix (because that usually + // separates the suffix from the double dash. + ArgsWithSuffix args("-foo -- "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), ""); +} + +TEST(ArgsWithSuffixTest, WhitespaceSuffix) { + // A single whtiespace character as a suffix. + ArgsWithSuffix args("-foo -- "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), " "); +} + +TEST(ArgsWithSuffixTest, LeadingSpaceArgs) { + // Whitespace before the first dash needs to be ignored. + ArgsWithSuffix args(" -foo -- bar"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), " -foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), " -foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), "bar"); +} + +TEST(ArgsWithSuffixTest, SingleWordSuffix) { + // A single word as a suffix. + ArgsWithSuffix args("-foo -- bar"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), "bar"); +} + +TEST(ArgsWithSuffixTest, MultiWordSuffix) { + // Multiple words as a suffix. + ArgsWithSuffix args("-foo -- bar baz"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), "bar baz"); +} + +TEST(ArgsWithSuffixTest, UnterminatedQuote) { + // A quote character in the suffix shouldn't influence the parsing. + ArgsWithSuffix args("-foo -- bar \" "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), "bar \" "); +} + +TEST(ArgsWithSuffixTest, TerminatedQuote) { + // A part of the suffix is quoted, which shouldn't influence the parsing. + ArgsWithSuffix args("-foo -- bar \"a\" "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetSuffix().c_str(), "bar \"a\" "); +} + +TEST(ArgsWithSuffixTest, EmptyArgsOnlySuffix) { + // Empty argument list, but we have a suffix. + ArgsWithSuffix args("-- bar"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), ""); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(0u, ref.size()); + + ASSERT_STREQ(args.GetSuffix().c_str(), "bar"); +} Index: source/Utility/Args.cpp =================================================================== --- source/Utility/Args.cpp +++ source/Utility/Args.cpp @@ -66,6 +66,13 @@ return count; } +// Trims all whitespace that can separate command line arguments from the left +// side of the string. +static llvm::StringRef ltrimForArgs(llvm::StringRef str) { + static const char *k_space_separators = " \t"; + return str.ltrim(k_space_separators); +} + // A helper function for SetCommandString. Parses a single argument from the // command string, processing quotes and backslashes in a shell-like manner. // The function returns a tuple consisting of the parsed argument, the quote @@ -237,15 +244,14 @@ Clear(); m_argv.clear(); - static const char *k_space_separators = " \t"; - command = command.ltrim(k_space_separators); + command = ltrimForArgs(command); std::string arg; char quote; while (!command.empty()) { std::tie(arg, quote, command) = ParseSingleArgument(command); m_entries.emplace_back(arg, quote); m_argv.push_back(m_entries.back().data()); - command = command.ltrim(k_space_separators); + command = ltrimForArgs(command); } m_argv.push_back(nullptr); } @@ -654,3 +660,59 @@ } return res; } + +ArgsWithSuffix::ArgsWithSuffix(llvm::StringRef arg_string) { + SetFromString(arg_string); +} + +void ArgsWithSuffix::SetFromString(llvm::StringRef arg_string) { + const llvm::StringRef original_args = arg_string; + + arg_string = ltrimForArgs(arg_string); + std::string arg; + char quote; + + if (!arg_string.startswith("-")) { + m_suffix = original_args; + return; + } + + bool found_suffix = false; + + while (!arg_string.empty()) { + // The length of the prefix before parsing. + std::size_t prev_prefix_length = original_args.size() - arg_string.size(); + + std::tie(arg, quote, arg_string) = ParseSingleArgument(arg_string); + + // If we get an unquoted '--' argument, then we reached the suffix part + // of the command. + Args::ArgEntry entry(arg, quote); + if (!entry.IsQuoted() && arg == "--") { + // The remaining line is the raw suffix, and the line we parsed so far + // needs to be interpreted as arguments. + m_has_args = true; + m_suffix = arg_string; + found_suffix = true; + + std::size_t prefix_length = original_args.size() - arg_string.size(); + + // Take the string we know contains all the arguments and actually parse + // it as proper arguments. + llvm::StringRef prefix = original_args.take_front(prev_prefix_length); + m_args = Args(prefix); + m_arg_string = prefix; + + m_arg_string_with_delimiter = original_args.take_front(prefix_length); + break; + } + + arg_string = ltrimForArgs(arg_string); + } + + // If we didn't find a suffix delimiter, the whole string is the raw suffix. + if (!found_suffix) { + found_suffix = true; + m_suffix = original_args; + } +} Index: source/Interpreter/Options.cpp =================================================================== --- source/Interpreter/Options.cpp +++ source/Interpreter/Options.cpp @@ -1334,7 +1334,7 @@ const Args::ArgEntry &cursor = args[cursor_index]; if ((static_cast<int32_t>(dash_dash_pos) == -1 || cursor_index < dash_dash_pos) && - cursor.quote == '\0' && cursor.ref == "-") { + !cursor.IsQuoted() && cursor.ref == "-") { option_element_vector.push_back( OptionArgElement(OptionArgElement::eBareDash, cursor_index, OptionArgElement::eBareDash)); Index: source/Interpreter/CommandObject.cpp =================================================================== --- source/Interpreter/CommandObject.cpp +++ source/Interpreter/CommandObject.cpp @@ -331,6 +331,22 @@ return found_word; } +bool CommandObject::ParseOptionsAndNotify(Args &args, + CommandReturnObject &result, + OptionGroupOptions &group_options, + ExecutionContext &exe_ctx) { + if (!ParseOptions(args, result)) + return false; + + Status error(group_options.NotifyOptionParsingFinished(&exe_ctx)); + if (error.Fail()) { + result.AppendError(error.AsCString()); + result.SetStatus(eReturnStatusFailed); + return false; + } + return true; +} + int CommandObject::GetNumArgumentEntries() { return m_arguments.size(); } CommandObject::CommandArgumentEntry * Index: source/Commands/CommandObjectWatchpoint.cpp =================================================================== --- source/Commands/CommandObjectWatchpoint.cpp +++ source/Commands/CommandObjectWatchpoint.cpp @@ -1035,46 +1035,18 @@ Target *target = m_interpreter.GetDebugger().GetSelectedTarget().get(); StackFrame *frame = m_exe_ctx.GetFramePtr(); - Args command(raw_command); - const char *expr = nullptr; - if (raw_command[0] == '-') { - // We have some options and these options MUST end with --. - const char *end_options = nullptr; - const char *s = raw_command; - while (s && s[0]) { - end_options = ::strstr(s, "--"); - if (end_options) { - end_options += 2; // Get past the "--" - if (::isspace(end_options[0])) { - expr = end_options; - while (::isspace(*expr)) - ++expr; - break; - } - } - s = end_options; - } + ArgsWithSuffix args(raw_command); - if (end_options) { - Args args(llvm::StringRef(raw_command, end_options - raw_command)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } + llvm::StringRef expr = args.GetSuffix(); - if (expr == nullptr) - expr = raw_command; + if (args.HasArgs()) + if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, + exe_ctx)) + return false; // If no argument is present, issue an error message. There's no way to // set a watchpoint. - if (command.GetArgumentCount() == 0) { + if (llvm::StringRef(raw_command).trim().empty()) { result.GetErrorStream().Printf("error: required argument missing; " "specify an expression to evaulate into " "the address to watch for\n"); @@ -1107,7 +1079,7 @@ if (expr_result != eExpressionCompleted) { result.GetErrorStream().Printf( "error: expression evaluation of address to watch failed\n"); - result.GetErrorStream().Printf("expression evaluated: %s\n", expr); + result.GetErrorStream() << "expression evaluated: \n" << expr << "\n"; result.SetStatus(eReturnStatusFailed); return false; } Index: source/Commands/CommandObjectType.cpp =================================================================== --- source/Commands/CommandObjectType.cpp +++ source/Commands/CommandObjectType.cpp @@ -2873,42 +2873,13 @@ auto exe_ctx = GetCommandInterpreter().GetExecutionContext(); m_option_group.NotifyOptionParsingStarting(&exe_ctx); - const char *name_of_type = nullptr; - - if (raw_command_line[0] == '-') { - // We have some options and these options MUST end with --. - const char *end_options = nullptr; - const char *s = raw_command_line; - while (s && s[0]) { - end_options = ::strstr(s, "--"); - if (end_options) { - end_options += 2; // Get past the "--" - if (::isspace(end_options[0])) { - name_of_type = end_options; - while (::isspace(*name_of_type)) - ++name_of_type; - break; - } - } - s = end_options; - } + ArgsWithSuffix args(raw_command_line); + const char *name_of_type = args.GetSuffix().c_str(); - if (end_options) { - Args args( - llvm::StringRef(raw_command_line, end_options - raw_command_line)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } - if (nullptr == name_of_type) - name_of_type = raw_command_line; + if (args.HasArgs()) + if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, + exe_ctx)) + return false; // TargetSP // target_sp(GetCommandInterpreter().GetDebugger().GetSelectedTarget()); Index: source/Commands/CommandObjectPlatform.cpp =================================================================== --- source/Commands/CommandObjectPlatform.cpp +++ source/Commands/CommandObjectPlatform.cpp @@ -1747,42 +1747,19 @@ ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext(); m_options.NotifyOptionParsingStarting(&exe_ctx); - const char *expr = nullptr; // Print out an usage syntax on an empty command line. if (raw_command_line[0] == '\0') { result.GetOutputStream().Printf("%s\n", this->GetSyntax().str().c_str()); return true; } - if (raw_command_line[0] == '-') { - // We have some options and these options MUST end with --. - const char *end_options = nullptr; - const char *s = raw_command_line; - while (s && s[0]) { - end_options = ::strstr(s, "--"); - if (end_options) { - end_options += 2; // Get past the "--" - if (::isspace(end_options[0])) { - expr = end_options; - while (::isspace(*expr)) - ++expr; - break; - } - } - s = end_options; - } + ArgsWithSuffix args(raw_command_line); + const char *expr = args.GetSuffix().c_str(); - if (end_options) { - Args args( - llvm::StringRef(raw_command_line, end_options - raw_command_line)); - if (!ParseOptions(args, result)) - return false; - } - } - - if (expr == nullptr) - expr = raw_command_line; + if (args.HasArgs()) + if (!ParseOptions(args.GetArgs(), result)) + return false; PlatformSP platform_sp( m_interpreter.GetDebugger().GetPlatformList().GetSelectedPlatform()); Index: source/Commands/CommandObjectExpression.cpp =================================================================== --- source/Commands/CommandObjectExpression.cpp +++ source/Commands/CommandObjectExpression.cpp @@ -514,111 +514,82 @@ auto exe_ctx = GetCommandInterpreter().GetExecutionContext(); m_option_group.NotifyOptionParsingStarting(&exe_ctx); - const char *expr = nullptr; - if (command[0] == '\0') { GetMultilineExpression(); return result.Succeeded(); } - if (command[0] == '-') { - // We have some options and these options MUST end with --. - const char *end_options = nullptr; - const char *s = command; - while (s && s[0]) { - end_options = ::strstr(s, "--"); - if (end_options) { - end_options += 2; // Get past the "--" - if (::isspace(end_options[0])) { - expr = end_options; - while (::isspace(*expr)) - ++expr; - break; - } - } - s = end_options; - } - - if (end_options) { - Args args(llvm::StringRef(command, end_options - command)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - - if (m_repl_option.GetOptionValue().GetCurrentValue()) { - Target *target = m_interpreter.GetExecutionContext().GetTargetPtr(); - if (target) { - // Drop into REPL - m_expr_lines.clear(); - m_expr_line_count = 0; - - Debugger &debugger = target->GetDebugger(); - - // Check if the LLDB command interpreter is sitting on top of a REPL - // that launched it... - if (debugger.CheckTopIOHandlerTypes( - IOHandler::Type::CommandInterpreter, IOHandler::Type::REPL)) { - // the LLDB command interpreter is sitting on top of a REPL that - // launched it, so just say the command interpreter is done and - // fall back to the existing REPL - m_interpreter.GetIOHandler(false)->SetIsDone(true); - } else { - // We are launching the REPL on top of the current LLDB command - // interpreter, so just push one - bool initialize = false; - Status repl_error; - REPLSP repl_sp(target->GetREPL( - repl_error, m_command_options.language, nullptr, false)); - - if (!repl_sp) { - initialize = true; - repl_sp = target->GetREPL(repl_error, m_command_options.language, - nullptr, true); - if (!repl_error.Success()) { - result.SetError(repl_error); - return result.Succeeded(); - } + ArgsWithSuffix args(command); + const char *expr = args.GetSuffix().c_str(); + + if (args.HasArgs()) { + if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, exe_ctx)) + return false; + + if (m_repl_option.GetOptionValue().GetCurrentValue()) { + Target *target = m_interpreter.GetExecutionContext().GetTargetPtr(); + if (target) { + // Drop into REPL + m_expr_lines.clear(); + m_expr_line_count = 0; + + Debugger &debugger = target->GetDebugger(); + + // Check if the LLDB command interpreter is sitting on top of a REPL + // that launched it... + if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter, + IOHandler::Type::REPL)) { + // the LLDB command interpreter is sitting on top of a REPL that + // launched it, so just say the command interpreter is done and + // fall back to the existing REPL + m_interpreter.GetIOHandler(false)->SetIsDone(true); + } else { + // We are launching the REPL on top of the current LLDB command + // interpreter, so just push one + bool initialize = false; + Status repl_error; + REPLSP repl_sp(target->GetREPL(repl_error, m_command_options.language, + nullptr, false)); + + if (!repl_sp) { + initialize = true; + repl_sp = target->GetREPL(repl_error, m_command_options.language, + nullptr, true); + if (!repl_error.Success()) { + result.SetError(repl_error); + return result.Succeeded(); } + } - if (repl_sp) { - if (initialize) { - repl_sp->SetCommandOptions(m_command_options); - repl_sp->SetFormatOptions(m_format_options); - repl_sp->SetValueObjectDisplayOptions(m_varobj_options); - } + if (repl_sp) { + if (initialize) { + repl_sp->SetCommandOptions(m_command_options); + repl_sp->SetFormatOptions(m_format_options); + repl_sp->SetValueObjectDisplayOptions(m_varobj_options); + } - IOHandlerSP io_handler_sp(repl_sp->GetIOHandler()); + IOHandlerSP io_handler_sp(repl_sp->GetIOHandler()); - io_handler_sp->SetIsDone(false); + io_handler_sp->SetIsDone(false); - debugger.PushIOHandler(io_handler_sp); - } else { - repl_error.SetErrorStringWithFormat( - "Couldn't create a REPL for %s", - Language::GetNameForLanguageType(m_command_options.language)); - result.SetError(repl_error); - return result.Succeeded(); - } + debugger.PushIOHandler(io_handler_sp); + } else { + repl_error.SetErrorStringWithFormat( + "Couldn't create a REPL for %s", + Language::GetNameForLanguageType(m_command_options.language)); + result.SetError(repl_error); + return result.Succeeded(); } } } - // No expression following options - else if (expr == nullptr || expr[0] == '\0') { - GetMultilineExpression(); - return result.Succeeded(); - } + } + // No expression following options + else if (expr[0] == '\0') { + GetMultilineExpression(); + return result.Succeeded(); } } - if (expr == nullptr) - expr = command; - Target *target = GetSelectedOrDummyTarget(); if (EvaluateExpression(expr, &(result.GetOutputStream()), &(result.GetErrorStream()), &result)) { @@ -629,13 +600,12 @@ // for expr???) // If we can it would be nice to show that. std::string fixed_command("expression "); - if (expr == command) - fixed_command.append(m_fixed_expression); - else { + if (args.HasArgs()) { // Add in any options that might have been in the original command: - fixed_command.append(command, expr - command); + fixed_command.append(args.GetArgStringWithDelimiter()); + fixed_command.append(m_fixed_expression); + } else fixed_command.append(m_fixed_expression); - } history.AppendString(fixed_command); } // Increment statistics to record this expression evaluation success. Index: source/Commands/CommandObjectCommands.cpp =================================================================== --- source/Commands/CommandObjectCommands.cpp +++ source/Commands/CommandObjectCommands.cpp @@ -553,42 +553,13 @@ ExecutionContext exe_ctx = GetCommandInterpreter().GetExecutionContext(); m_option_group.NotifyOptionParsingStarting(&exe_ctx); - const char *remainder = nullptr; - - if (raw_command_line[0] == '-') { - // We have some options and these options MUST end with --. - const char *end_options = nullptr; - const char *s = raw_command_line; - while (s && s[0]) { - end_options = ::strstr(s, "--"); - if (end_options) { - end_options += 2; // Get past the "--" - if (::isspace(end_options[0])) { - remainder = end_options; - while (::isspace(*remainder)) - ++remainder; - break; - } - } - s = end_options; - } + ArgsWithSuffix args_with_suffix(raw_command_line); + const char *remainder = args_with_suffix.GetSuffix().c_str(); - if (end_options) { - Args args( - llvm::StringRef(raw_command_line, end_options - raw_command_line)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } - if (nullptr == remainder) - remainder = raw_command_line; + if (args_with_suffix.HasArgs()) + if (!ParseOptionsAndNotify(args_with_suffix.GetArgs(), result, + m_option_group, exe_ctx)) + return false; llvm::StringRef raw_command_string(remainder); Args args(raw_command_string); Index: include/lldb/Utility/Args.h =================================================================== --- include/lldb/Utility/Args.h +++ include/lldb/Utility/Args.h @@ -48,6 +48,11 @@ llvm::StringRef ref; char quote; const char *c_str() const { return ptr.get(); } + + //------------------------------------------------------------------ + /// Returns true if this argument was quoted in any way. + //------------------------------------------------------------------ + bool IsQuoted() const { return quote != '\0'; } }; //------------------------------------------------------------------ @@ -353,6 +358,105 @@ std::vector<char *> m_argv; }; +//---------------------------------------------------------------------- +/// @class ArgsWithSuffix Args.h "lldb/Utility/Args.h" +/// A pair of an argument list with a 'raw' string as a suffix. +/// +/// This class works like Args, but handles the case where we have a trailing +/// string that shouldn't be interpreted as a list of arguments but preserved +/// as is. +/// +/// The leading arguments are optional. If the first non-space character +/// in the string starts with a dash, and the string contains an argument +/// that is an unquoted double dash (' -- '), then everything up to the double +/// dash is parsed as a list of arguments. Everything after the double dash +/// is interpreted as the raw suffix string. Note that the spaces behind the +/// double dash is not part of the raw suffix. +/// +/// All strings not matching the above format as considered to be just a raw +/// suffix without any arguments. +/// +/// @see Args +//---------------------------------------------------------------------- +class ArgsWithSuffix { +public: + //------------------------------------------------------------------ + /// Parse the given string as a list of optional arguments with a raw suffix. + /// + /// See the class description for a description of the input format. + /// + /// @param[in] argument_string + /// The string that should be parsed. + //------------------------------------------------------------------ + explicit ArgsWithSuffix(llvm::StringRef argument_string); + + //------------------------------------------------------------------ + /// Returns true if there are any arguments before the raw suffix. + //------------------------------------------------------------------ + bool HasArgs() const { return m_has_args; } + + //------------------------------------------------------------------ + /// Returns the list of arguments. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + Args &GetArgs() { + assert(m_has_args); + return m_args; + } + + //------------------------------------------------------------------ + /// Returns the list of arguments. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + const Args &GetArgs() const { + assert(m_has_args); + return m_args; + } + + //------------------------------------------------------------------ + /// Returns the part of the input string that was used for parsing the + /// argument list. This string also includes the double dash that is used + /// for separating the argument list from the suffix. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + llvm::StringRef GetArgStringWithDelimiter() const { + assert(m_has_args); + return m_arg_string_with_delimiter; + } + + //------------------------------------------------------------------ + /// Returns the part of the input string that was used for parsing the + /// argument list. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + llvm::StringRef GetArgString() const { + assert(m_has_args); + return m_arg_string; + } + + //------------------------------------------------------------------ + /// Returns the raw suffix part of the parsed string. + //------------------------------------------------------------------ + const std::string &GetSuffix() const { return m_suffix; } + +private: + void SetFromString(llvm::StringRef arg_string); + + /// Keeps track if we have parsed and stored any arguments. + bool m_has_args = false; + Args m_args; + llvm::StringRef m_arg_string; + llvm::StringRef m_arg_string_with_delimiter; + + // FIXME: This should be a StringRef, but some of the calling code expect a + // C string here so only a real std::string is possible. + std::string m_suffix; +}; + } // namespace lldb_private #endif // LLDB_UTILITY_ARGS_H Index: include/lldb/Interpreter/CommandObject.h =================================================================== --- include/lldb/Interpreter/CommandObject.h +++ include/lldb/Interpreter/CommandObject.h @@ -337,6 +337,10 @@ CommandReturnObject &result) = 0; protected: + bool ParseOptionsAndNotify(Args &args, CommandReturnObject &result, + OptionGroupOptions &group_options, + ExecutionContext &exe_ctx); + virtual const char *GetInvalidTargetDescription() { return "invalid target, create a target using the 'target create' command"; }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits