teemperor updated this revision to Diff 154862. teemperor retitled this revision from "Refactor parsing of argument lists with a raw string suffix." to "Refactor parsing of option lists with a raw string suffix.". teemperor edited the summary of this revision. teemperor added a comment.
- Renamed class to OptionsWithRaw - Changed documentation a bit that it becomes clear this class is more about about parsing command line options. @jingham Yeah, the class became more option specific while I was trying to get the existing code working. I'm not sure if we have a way to improve the parsing, as depending on the command the input is very ambiguous (e.g. `expr -i0 * -- i` can really be both from the point of the parser). The current code just keeps the behavior of the existing code (which is ok, as that's not the thing the patch wants to fix). 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/CMakeLists.txt unittests/Utility/OptionsWithRawTest.cpp
Index: unittests/Utility/OptionsWithRawTest.cpp =================================================================== --- /dev/null +++ unittests/Utility/OptionsWithRawTest.cpp @@ -0,0 +1,183 @@ +//===-- OptionsWithRawTest.cpp ----------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "gtest/gtest.h" + +#include "lldb/Utility/Args.h" +#include "lldb/Utility/StringList.h" + +using namespace lldb_private; + +TEST(OptionsWithRawTest, EmptyInput) { + // An empty string is just an empty suffix without any arguments. + OptionsWithRaw args(""); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetRawPart().c_str(), ""); +} + +TEST(OptionsWithRawTest, SingleWhitespaceInput) { + // Only whitespace is just a suffix. + OptionsWithRaw args(" "); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetRawPart().c_str(), " "); +} + +TEST(OptionsWithRawTest, WhitespaceInput) { + // Only whitespace is just a suffix. + OptionsWithRaw args(" "); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetRawPart().c_str(), " "); +} + +TEST(OptionsWithRawTest, ArgsButNoDelimiter) { + // This counts as a suffix because there is no -- at the end. + OptionsWithRaw args("-foo bar"); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetRawPart().c_str(), "-foo bar"); +} + +TEST(OptionsWithRawTest, ArgsButNoLeadingDash) { + // No leading dash means we have no arguments. + OptionsWithRaw args("foo bar --"); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetRawPart().c_str(), "foo bar --"); +} + +TEST(OptionsWithRawTest, QuotedSuffix) { + // We need to have a way to escape the -- to make it usable as an argument. + OptionsWithRaw args("-foo \"--\" bar"); + ASSERT_FALSE(args.HasArgs()); + ASSERT_STREQ(args.GetRawPart().c_str(), "-foo \"--\" bar"); +} + +TEST(OptionsWithRawTest, EmptySuffix) { + // An empty suffix with arguments. + OptionsWithRaw args("-foo --"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo --"); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), ""); +} + +TEST(OptionsWithRawTest, EmptySuffixSingleWhitespace) { + // A single whitespace also countas as an empty suffix (because that usually + // separates the suffix from the double dash. + OptionsWithRaw args("-foo -- "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), ""); +} + +TEST(OptionsWithRawTest, WhitespaceSuffix) { + // A single whtiespace character as a suffix. + OptionsWithRaw args("-foo -- "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), " "); +} + +TEST(OptionsWithRawTest, LeadingSpaceArgs) { + // Whitespace before the first dash needs to be ignored. + OptionsWithRaw args(" -foo -- bar"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), " -foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), " -foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), "bar"); +} + +TEST(OptionsWithRawTest, SingleWordSuffix) { + // A single word as a suffix. + OptionsWithRaw args("-foo -- bar"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), "bar"); +} + +TEST(OptionsWithRawTest, MultiWordSuffix) { + // Multiple words as a suffix. + OptionsWithRaw args("-foo -- bar baz"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), "bar baz"); +} + +TEST(OptionsWithRawTest, UnterminatedQuote) { + // A quote character in the suffix shouldn't influence the parsing. + OptionsWithRaw args("-foo -- bar \" "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), "bar \" "); +} + +TEST(OptionsWithRawTest, TerminatedQuote) { + // A part of the suffix is quoted, which shouldn't influence the parsing. + OptionsWithRaw args("-foo -- bar \"a\" "); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), "-foo "); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-foo -- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(1u, ref.size()); + EXPECT_STREQ("-foo", ref[0]); + + ASSERT_STREQ(args.GetRawPart().c_str(), "bar \"a\" "); +} + +TEST(OptionsWithRawTest, EmptyArgsOnlySuffix) { + // Empty argument list, but we have a suffix. + OptionsWithRaw args("-- bar"); + ASSERT_TRUE(args.HasArgs()); + ASSERT_EQ(args.GetArgString(), ""); + ASSERT_EQ(args.GetArgStringWithDelimiter(), "-- "); + + auto ref = args.GetArgs().GetArgumentArrayRef(); + ASSERT_EQ(0u, ref.size()); + + ASSERT_STREQ(args.GetRawPart().c_str(), "bar"); +} Index: unittests/Utility/CMakeLists.txt =================================================================== --- unittests/Utility/CMakeLists.txt +++ unittests/Utility/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(UtilityTests ArgsTest.cpp + OptionsWithRawTest.cpp ArchSpecTest.cpp CleanUpTest.cpp ConstStringTest.cpp 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,67 @@ } return res; } + +OptionsWithRaw::OptionsWithRaw(llvm::StringRef arg_string) { + SetFromString(arg_string); +} + +void OptionsWithRaw::SetFromString(llvm::StringRef arg_string) { + const llvm::StringRef original_args = arg_string; + + arg_string = ltrimForArgs(arg_string); + std::string arg; + char quote; + + // If the string doesn't start with a dash, we just have no options and just + // a raw part. + if (!arg_string.startswith("-")) { + m_suffix = original_args; + return; + } + + bool found_suffix = false; + + while (!arg_string.empty()) { + // The length of the prefix before parsing. + std::size_t prev_prefix_length = original_args.size() - arg_string.size(); + + // Parse the next argument from the remaining string. + std::tie(arg, quote, arg_string) = ParseSingleArgument(arg_string); + + // If we get an unquoted '--' argument, then we reached the suffix part + // of the command. + Args::ArgEntry entry(arg, quote); + if (!entry.IsQuoted() && arg == "--") { + // The remaining line is the raw suffix, and the line we parsed so far + // needs to be interpreted as arguments. + m_has_args = true; + m_suffix = arg_string; + found_suffix = true; + + // The length of the prefix after parsing. + std::size_t prefix_length = original_args.size() - arg_string.size(); + + // Take the string we know contains all the arguments and actually parse + // it as proper arguments. + llvm::StringRef prefix = original_args.take_front(prev_prefix_length); + m_args = Args(prefix); + m_arg_string = prefix; + + // We also record the part of the string that contains the arguments plus + // the delimiter. + m_arg_string_with_delimiter = original_args.take_front(prefix_length); + + // As the rest of the string became the raw suffix, we are done here. + break; + } + + arg_string = ltrimForArgs(arg_string); + } + + // If we didn't find a suffix delimiter, the whole string is the raw suffix. + if (!found_suffix) { + found_suffix = true; + m_suffix = original_args; + } +} 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; - } + OptionsWithRaw args(raw_command); - if (end_options) { - Args args(llvm::StringRef(raw_command, end_options - raw_command)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } + llvm::StringRef expr = args.GetRawPart(); - if (expr == nullptr) - expr = raw_command; + if (args.HasArgs()) + if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, + exe_ctx)) + return false; // If no argument is present, issue an error message. There's no way to // set a watchpoint. - if (command.GetArgumentCount() == 0) { + if (llvm::StringRef(raw_command).trim().empty()) { result.GetErrorStream().Printf("error: required argument missing; " "specify an expression to evaulate into " "the address to watch for\n"); @@ -1107,7 +1079,7 @@ 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; - } + OptionsWithRaw args(raw_command_line); + const char *name_of_type = args.GetRawPart().c_str(); - if (end_options) { - Args args( - llvm::StringRef(raw_command_line, end_options - raw_command_line)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } - if (nullptr == name_of_type) - name_of_type = raw_command_line; + if (args.HasArgs()) + if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, + exe_ctx)) + return false; // TargetSP // target_sp(GetCommandInterpreter().GetDebugger().GetSelectedTarget()); 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; - } + OptionsWithRaw args(raw_command_line); + const char *expr = args.GetRawPart().c_str(); - if (end_options) { - Args args( - llvm::StringRef(raw_command_line, end_options - raw_command_line)); - if (!ParseOptions(args, result)) - return false; - } - } - - if (expr == nullptr) - expr = raw_command_line; + if (args.HasArgs()) + if (!ParseOptions(args.GetArgs(), result)) + return false; PlatformSP platform_sp( m_interpreter.GetDebugger().GetPlatformList().GetSelectedPlatform()); 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(); - } + OptionsWithRaw args(command); + const char *expr = args.GetRawPart().c_str(); + + if (args.HasArgs()) { + if (!ParseOptionsAndNotify(args.GetArgs(), result, m_option_group, exe_ctx)) + return false; + + if (m_repl_option.GetOptionValue().GetCurrentValue()) { + Target *target = m_interpreter.GetExecutionContext().GetTargetPtr(); + if (target) { + // Drop into REPL + m_expr_lines.clear(); + m_expr_line_count = 0; + + Debugger &debugger = target->GetDebugger(); + + // Check if the LLDB command interpreter is sitting on top of a REPL + // that launched it... + if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter, + IOHandler::Type::REPL)) { + // the LLDB command interpreter is sitting on top of a REPL that + // launched it, so just say the command interpreter is done and + // fall back to the existing REPL + m_interpreter.GetIOHandler(false)->SetIsDone(true); + } else { + // We are launching the REPL on top of the current LLDB command + // interpreter, so just push one + bool initialize = false; + Status repl_error; + REPLSP repl_sp(target->GetREPL(repl_error, m_command_options.language, + nullptr, false)); + + if (!repl_sp) { + initialize = true; + repl_sp = target->GetREPL(repl_error, m_command_options.language, + nullptr, true); + if (!repl_error.Success()) { + result.SetError(repl_error); + return result.Succeeded(); } + } - if (repl_sp) { - if (initialize) { - repl_sp->SetCommandOptions(m_command_options); - repl_sp->SetFormatOptions(m_format_options); - repl_sp->SetValueObjectDisplayOptions(m_varobj_options); - } + if (repl_sp) { + if (initialize) { + repl_sp->SetCommandOptions(m_command_options); + repl_sp->SetFormatOptions(m_format_options); + repl_sp->SetValueObjectDisplayOptions(m_varobj_options); + } - IOHandlerSP io_handler_sp(repl_sp->GetIOHandler()); + IOHandlerSP io_handler_sp(repl_sp->GetIOHandler()); - io_handler_sp->SetIsDone(false); + io_handler_sp->SetIsDone(false); - debugger.PushIOHandler(io_handler_sp); - } else { - repl_error.SetErrorStringWithFormat( - "Couldn't create a REPL for %s", - Language::GetNameForLanguageType(m_command_options.language)); - result.SetError(repl_error); - return result.Succeeded(); - } + debugger.PushIOHandler(io_handler_sp); + } else { + repl_error.SetErrorStringWithFormat( + "Couldn't create a REPL for %s", + Language::GetNameForLanguageType(m_command_options.language)); + result.SetError(repl_error); + return result.Succeeded(); } } } - // No expression following options - else if (expr == nullptr || expr[0] == '\0') { - GetMultilineExpression(); - return result.Succeeded(); - } + } + // No expression following options + else if (expr[0] == '\0') { + GetMultilineExpression(); + return result.Succeeded(); } } - if (expr == nullptr) - expr = command; - Target *target = GetSelectedOrDummyTarget(); if (EvaluateExpression(expr, &(result.GetOutputStream()), &(result.GetErrorStream()), &result)) { @@ -629,13 +600,12 @@ // 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; - } + OptionsWithRaw args_with_suffix(raw_command_line); + const char *remainder = args_with_suffix.GetRawPart().c_str(); - if (end_options) { - Args args( - llvm::StringRef(raw_command_line, end_options - raw_command_line)); - if (!ParseOptions(args, result)) - return false; - - Status error(m_option_group.NotifyOptionParsingFinished(&exe_ctx)); - if (error.Fail()) { - result.AppendError(error.AsCString()); - result.SetStatus(eReturnStatusFailed); - return false; - } - } - } - if (nullptr == remainder) - remainder = raw_command_line; + if (args_with_suffix.HasArgs()) + if (!ParseOptionsAndNotify(args_with_suffix.GetArgs(), result, + m_option_group, exe_ctx)) + return false; llvm::StringRef raw_command_string(remainder); Args args(raw_command_string); 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,106 @@ std::vector<char *> m_argv; }; +//---------------------------------------------------------------------- +/// @class OptionsWithRaw Args.h "lldb/Utility/Args.h" +/// A pair of an option list with a 'raw' string as a suffix. +/// +/// This class works similar to Args, but handles the case where we have a +/// trailing string that shouldn't be interpreted as a list of arguments but +/// preserved as is. It is also only useful for handling command line options +/// (e.g. '-foo bar -i0') that start with a dash. +/// +/// The leading option list is optional. If the first non-space character +/// in the string starts with a dash, and the string contains an argument +/// that is an unquoted double dash (' -- '), then everything up to the double +/// dash is parsed as a list of arguments. Everything after the double dash +/// is interpreted as the raw suffix string. Note that the space behind the +/// double dash is not part of the raw suffix. +/// +/// All strings not matching the above format as considered to be just a raw +/// string without any options. +/// +/// @see Args +//---------------------------------------------------------------------- +class OptionsWithRaw { +public: + //------------------------------------------------------------------ + /// Parse the given string as a list of optional arguments with a raw suffix. + /// + /// See the class description for a description of the input format. + /// + /// @param[in] argument_string + /// The string that should be parsed. + //------------------------------------------------------------------ + explicit OptionsWithRaw(llvm::StringRef argument_string); + + //------------------------------------------------------------------ + /// Returns true if there are any arguments before the raw suffix. + //------------------------------------------------------------------ + bool HasArgs() const { return m_has_args; } + + //------------------------------------------------------------------ + /// Returns the list of arguments. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + Args &GetArgs() { + assert(m_has_args); + return m_args; + } + + //------------------------------------------------------------------ + /// Returns the list of arguments. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + const Args &GetArgs() const { + assert(m_has_args); + return m_args; + } + + //------------------------------------------------------------------ + /// Returns the part of the input string that was used for parsing the + /// argument list. This string also includes the double dash that is used + /// for separating the argument list from the suffix. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + llvm::StringRef GetArgStringWithDelimiter() const { + assert(m_has_args); + return m_arg_string_with_delimiter; + } + + //------------------------------------------------------------------ + /// Returns the part of the input string that was used for parsing the + /// argument list. + /// + /// You can only call this method if HasArgs returns true. + //------------------------------------------------------------------ + llvm::StringRef GetArgString() const { + assert(m_has_args); + return m_arg_string; + } + + //------------------------------------------------------------------ + /// Returns the raw suffix part of the parsed string. + //------------------------------------------------------------------ + const std::string &GetRawPart() const { return m_suffix; } + +private: + void SetFromString(llvm::StringRef arg_string); + + /// Keeps track if we have parsed and stored any arguments. + bool m_has_args = false; + Args m_args; + llvm::StringRef m_arg_string; + llvm::StringRef m_arg_string_with_delimiter; + + // FIXME: This should be a StringRef, but some of the calling code expect a + // C string here so only a real std::string is possible. + std::string m_suffix; +}; + } // namespace lldb_private #endif // LLDB_UTILITY_ARGS_H 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