[Lldb-commits] [lldb] ac05bc0 - Revert "Be more careful to maintain quoting information when parsing commands."
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 13 15:02:29 PDT 2022
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
-}
-
More information about the lldb-commits
mailing list