Author: Jim Ingham Date: 2022-09-13T14:59:21-07:00 New Revision: ac05bc0524c66c74278b26742896a4c634c034cf
URL: https://github.com/llvm/llvm-project/commit/ac05bc0524c66c74278b26742896a4c634c034cf DIFF: https://github.com/llvm/llvm-project/commit/ac05bc0524c66c74278b26742896a4c634c034cf.diff LOG: Revert "Be more careful to maintain quoting information when parsing commands." This reverts commit 6c089b2af5d8d98f66b27b67f70958f520820a76. This was causing the test test_help_run_hides_options from TestHelp.py to fail on Linux and Windows (but the test succeeds on macOS). The decision to print option information is determined by CommandObjectAlias::IsDashDashCommand which was changed, but only by replacing an inline string constant with a const char * CommandInterpreter::g_argument which has the same string value. I can't see why this would fail, I'll have to spin up a vm to see if I can repo there. Added: Modified: lldb/include/lldb/Interpreter/CommandInterpreter.h lldb/packages/Python/lldbsuite/test/lldbtest.py lldb/source/Interpreter/CommandAlias.cpp lldb/source/Interpreter/CommandInterpreter.cpp lldb/source/Interpreter/CommandObject.cpp lldb/source/Interpreter/Options.cpp lldb/source/Utility/Args.cpp Removed: lldb/test/API/commands/command/backticks/Makefile lldb/test/API/commands/command/backticks/TestBackticksInAlias.py lldb/test/API/commands/command/backticks/main.c ################################################################################ diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 71587eed68c9..0f137a7b3c49 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -239,14 +239,6 @@ class CommandInterpreter : public Broadcaster, eCommandTypesAllThem = 0xFFFF //< all commands }; - // The CommandAlias and CommandInterpreter both have a hand in - // substituting for alias commands. They work by writing special tokens - // in the template form of the Alias command, and then detecting them when the - // command is executed. These are the special tokens: - static const char * const g_no_argument; - static const char * const g_need_argument; - static const char * const g_argument; - CommandInterpreter(Debugger &debugger, bool synchronous_execution); ~CommandInterpreter() override = default; diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py index 5c058769ff3f..101921ffc768 100644 --- a/lldb/packages/Python/lldbsuite/test/lldbtest.py +++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py @@ -2472,13 +2472,6 @@ def assertSuccess(self, obj, msg=None): self.fail(self._formatMessage(msg, "'{}' is not success".format(error))) - """Assert that a command return object is successful""" - def assertCommandReturn(self, obj, msg=None): - if not obj.Succeeded(): - error = obj.GetError() - self.fail(self._formatMessage(msg, - "'{}' is not success".format(error))) - """Assert two states are equal""" def assertState(self, first, second, msg=None): if first != second: diff --git a/lldb/source/Interpreter/CommandAlias.cpp b/lldb/source/Interpreter/CommandAlias.cpp index e36edcac15a5..d55b3fdd44fa 100644 --- a/lldb/source/Interpreter/CommandAlias.cpp +++ b/lldb/source/Interpreter/CommandAlias.cpp @@ -60,12 +60,11 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp, if (!options_string.empty()) { if (cmd_obj_sp->WantsRawCommandString()) - option_arg_vector->emplace_back(CommandInterpreter::g_argument, - -1, options_string); + option_arg_vector->emplace_back("<argument>", -1, options_string); else { for (auto &entry : args.entries()) { if (!entry.ref().empty()) - option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1, + option_arg_vector->emplace_back(std::string("<argument>"), -1, std::string(entry.ref())); } } @@ -154,12 +153,11 @@ void CommandAlias::GetAliasExpansion(StreamString &help_string) const { for (const auto &opt_entry : *options) { std::tie(opt, std::ignore, value) = opt_entry; - if (opt == CommandInterpreter::g_argument) { + if (opt == "<argument>") { help_string.Printf(" %s", value.c_str()); } else { help_string.Printf(" %s", opt.c_str()); - if ((value != CommandInterpreter::g_no_argument) - && (value != CommandInterpreter::g_need_argument)) { + if ((value != "<no-argument>") && (value != "<need-argument")) { help_string.Printf(" %s", value.c_str()); } } @@ -180,7 +178,7 @@ bool CommandAlias::IsDashDashCommand() { for (const auto &opt_entry : *GetOptionArguments()) { std::tie(opt, std::ignore, value) = opt_entry; - if (opt == CommandInterpreter::g_argument && !value.empty() && + if (opt == "<argument>" && !value.empty() && llvm::StringRef(value).endswith("--")) { m_is_dashdash_alias = eLazyBoolYes; break; @@ -208,8 +206,6 @@ std::pair<lldb::CommandObjectSP, OptionArgVectorSP> CommandAlias::Desugar() { return {nullptr, nullptr}; if (underlying->IsAlias()) { - // FIXME: This doesn't work if the original alias fills a slot in the - // underlying alias, since this just appends the two lists. auto desugared = ((CommandAlias *)underlying.get())->Desugar(); auto options = GetOptionArguments(); options->insert(options->begin(), desugared.second->begin(), diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 2218d54e3d97..0596d85bf21b 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -105,11 +105,6 @@ static constexpr const char *InitFileWarning = "and\n" "accept the security risk."; -constexpr const char *CommandInterpreter::g_no_argument = "<no-argument>"; -constexpr const char *CommandInterpreter::g_need_argument = "<need-argument>"; -constexpr const char *CommandInterpreter::g_argument = "<argument>"; - - #define LLDB_PROPERTIES_interpreter #include "InterpreterProperties.inc" @@ -1639,7 +1634,7 @@ CommandObject *CommandInterpreter::BuildAliasResult( std::string value; for (const auto &entry : *option_arg_vector) { std::tie(option, value_type, value) = entry; - if (option == g_argument) { + if (option == "<argument>") { result_str.Printf(" %s", value.c_str()); continue; } @@ -1661,33 +1656,11 @@ CommandObject *CommandInterpreter::BuildAliasResult( index); return nullptr; } else { - const Args::ArgEntry &entry = cmd_args[index]; - size_t strpos = raw_input_string.find(entry.c_str()); - const char quote_char = entry.GetQuoteChar(); - if (strpos != std::string::npos) { - const size_t start_fudge = quote_char == '\0' ? 0 : 1; - const size_t len_fudge = quote_char == '\0' ? 0 : 2; - - // Make sure we aren't going outside the bounds of the cmd string: - if (strpos < start_fudge) { - result.AppendError("Unmatched quote at command beginning."); - return nullptr; - } - llvm::StringRef arg_text = entry.ref(); - if (strpos - start_fudge + arg_text.size() + len_fudge - > raw_input_string.size()) { - result.AppendError("Unmatched quote at command end."); - return nullptr; - } + size_t strpos = raw_input_string.find(cmd_args.GetArgumentAtIndex(index)); + if (strpos != std::string::npos) raw_input_string = raw_input_string.erase( - strpos - start_fudge, - strlen(cmd_args.GetArgumentAtIndex(index)) + len_fudge); - } - if (quote_char == '\0') - result_str.Printf("%s", cmd_args.GetArgumentAtIndex(index)); - else - result_str.Printf("%c%s%c", quote_char, - entry.c_str(), quote_char); + strpos, strlen(cmd_args.GetArgumentAtIndex(index))); + result_str.Printf("%s", cmd_args.GetArgumentAtIndex(index)); } } @@ -1938,6 +1911,13 @@ bool CommandInterpreter::HandleCommand(const char *command_line, return true; } + Status error(PreprocessCommand(command_string)); + + if (error.Fail()) { + result.AppendError(error.AsCString()); + return false; + } + // Phase 1. // Before we do ANY kind of argument processing, we need to figure out what @@ -1955,20 +1935,6 @@ bool CommandInterpreter::HandleCommand(const char *command_line, CommandObject *cmd_obj = ResolveCommandImpl(command_string, result); - // We have to preprocess the whole command string for Raw commands, since we - // don't know the structure of the command. For parsed commands, we only - // treat backticks as quote characters specially. - // FIXME: We probably want to have raw commands do their own preprocessing. - // For instance, I don't think people expect substitution in expr expressions. - if (cmd_obj && cmd_obj->WantsRawCommandString()) { - Status error(PreprocessCommand(command_string)); - - if (error.Fail()) { - result.AppendError(error.AsCString()); - return false; - } - } - // Although the user may have abbreviated the command, the command_string now // has the command expanded to the full name. For example, if the input was // "br s -n main", command_string is now "breakpoint set -n main". @@ -2197,7 +2163,7 @@ void CommandInterpreter::BuildAliasCommandArgs(CommandObject *alias_cmd_obj, std::string value; for (const auto &option_entry : *option_arg_vector) { std::tie(option, value_type, value) = option_entry; - if (option == g_argument) { + if (option == "<argument>") { if (!wants_raw_input || (value != "--")) { // Since we inserted this above, make sure we don't insert it twice new_args.AppendArgument(value); @@ -2208,7 +2174,7 @@ void CommandInterpreter::BuildAliasCommandArgs(CommandObject *alias_cmd_obj, if (value_type != OptionParser::eOptionalArgument) new_args.AppendArgument(option); - if (value == g_no_argument) + if (value == "<no-argument>") continue; int index = GetOptionArgumentPosition(value.c_str()); diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp index 4500378c6229..5ab3392040c9 100644 --- a/lldb/source/Interpreter/CommandObject.cpp +++ b/lldb/source/Interpreter/CommandObject.cpp @@ -727,7 +727,7 @@ bool CommandObjectParsed::Execute(const char *args_string, } if (!handled) { for (auto entry : llvm::enumerate(cmd_args.entries())) { - if (!entry.value().ref().empty() && entry.value().GetQuoteChar() == '`') { + if (!entry.value().ref().empty() && entry.value().ref().front() == '`') { cmd_args.ReplaceArgumentAtIndex( entry.index(), m_interpreter.ProcessEmbeddedScriptCommands(entry.value().c_str())); diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index ae3625012c79..09af51a48212 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -1002,47 +1002,37 @@ llvm::Expected<Args> Options::ParseAlias(const Args &args, .str(), llvm::inconvertibleErrorCode()); } + if (!option_arg) + option_arg = "<no-argument>"; + option_arg_vector->emplace_back(std::string(option_str.GetString()), + has_arg, std::string(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. - // Note: We also need to preserve any option argument values that were - // surrounded by backticks, as we lose track of them in the - // option_args_vector. size_t idx = FindArgumentIndexForOption(args_copy, long_options[long_options_index]); - std::string option_to_insert; - if (option_arg) { - if (idx != size_t(-1) && has_arg) { - bool arg_has_backtick = args_copy[idx + 1].GetQuoteChar() == '`'; - if (arg_has_backtick) - option_to_insert = "`"; - option_to_insert += option_arg; - if (arg_has_backtick) - option_to_insert += "`"; - } else - option_to_insert = option_arg; - } else - option_to_insert = CommandInterpreter::g_no_argument; - - option_arg_vector->emplace_back(std::string(option_str.GetString()), - has_arg, option_to_insert); - if (idx == size_t(-1)) continue; if (!input_line.empty()) { - llvm::StringRef tmp_arg = args_copy[idx].ref(); + auto tmp_arg = args_copy[idx].ref(); size_t pos = input_line.find(std::string(tmp_arg)); if (pos != std::string::npos) input_line.erase(pos, tmp_arg.size()); } args_copy.DeleteArgumentAtIndex(idx); - if (option_to_insert != CommandInterpreter::g_no_argument) { + 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) { - size_t pos = input_line.find(option_to_insert); + auto tmp_arg = args_copy[idx].ref(); + size_t pos = input_line.find(std::string(tmp_arg)); if (pos != std::string::npos) - input_line.erase(pos, option_to_insert.size()); + input_line.erase(pos, tmp_arg.size()); } args_copy.DeleteArgumentAtIndex(idx); } diff --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp index 42c84d7a0456..daccb91d8436 100644 --- a/lldb/source/Utility/Args.cpp +++ b/lldb/source/Utility/Args.cpp @@ -215,12 +215,7 @@ bool Args::GetCommandString(std::string &command) const { for (size_t i = 0; i < m_entries.size(); ++i) { if (i > 0) command += ' '; - char quote = m_entries[i].quote; - if (quote != '\0') - command += quote; command += m_entries[i].ref(); - if (quote != '\0') - command += quote; } return !m_entries.empty(); diff --git a/lldb/test/API/commands/command/backticks/Makefile b/lldb/test/API/commands/command/backticks/Makefile deleted file mode 100644 index 10495940055b..000000000000 --- a/lldb/test/API/commands/command/backticks/Makefile +++ /dev/null @@ -1,3 +0,0 @@ -C_SOURCES := main.c - -include Makefile.rules diff --git a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py b/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py deleted file mode 100644 index 495e52db53aa..000000000000 --- a/lldb/test/API/commands/command/backticks/TestBackticksInAlias.py +++ /dev/null @@ -1,32 +0,0 @@ -""" -Test that an alias can contain active backticks -""" - - - -import lldb -from lldbsuite.test.lldbtest import * -import lldbsuite.test.lldbutil as lldbutil - - -class TestBackticksInAlias(TestBase): - NO_DEBUG_INFO_TESTCASE = True - - def test_backticks_in_alias(self): - """Test that an alias can contain active backticks.""" - self.build() - (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) - interp = self.dbg.GetCommandInterpreter(); - result = lldb.SBCommandReturnObject() - interp.HandleCommand('command alias _test-argv-cmd expression -Z \`argc\` -- argv', result) - self.assertCommandReturn(result, "Made the alias") - interp.HandleCommand("_test-argv-cmd", result) - self.assertCommandReturn(result, "The alias worked") - - # Now try a harder case where we create this using an alias: - interp.HandleCommand('command alias _test-argv-parray-cmd parray \`argc\` argv', result) - self.assertCommandReturn(result, "Made the alias") - interp.HandleCommand("_test-argv-parray-cmd", result) - self.assertFalse(result.Succeeded(), "CommandAlias::Desugar currently fails if a alias substitutes %N arguments in another alias") - - diff --git a/lldb/test/API/commands/command/backticks/main.c b/lldb/test/API/commands/command/backticks/main.c deleted file mode 100644 index a3abaaa6e8a1..000000000000 --- a/lldb/test/API/commands/command/backticks/main.c +++ /dev/null @@ -1,5 +0,0 @@ -int main (int argc, char const *argv[]) -{ - return argc; // break here -} - _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits