Revert "Revert "Be more careful to maintain quoting information when parsing commands.""

This reverts commit ac05bc0524.

I had incorrectly removed one set of checks in the option handling in
Options::ParseAlias because I couldn't see what it is for.  It was a
bit obscure, but it handled the case where you pass "-something=other --"
as the input_line, which caused the built-in "run" alias not to return
the right value for IsDashDashCommand, causing TestHelp.py to fail.

lldb/include/lldb/Interpreter/CommandInterpreter.h

@@ -239,6 +239,14 @@ public:
     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 *g_no_argument;
+  static const char *g_need_argument;
+  static const char *g_argument;
+
   CommandInterpreter(Debugger &debugger, bool synchronous_execution);
 
   ~CommandInterpreter() override = default;

lldb/packages/Python/lldbsuite/test/lldbtest.py

@@ -2472,6 +2472,13 @@ FileCheck output:
             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:

lldb/source/Interpreter/CommandAlias.cpp

@@ -60,11 +60,12 @@ static bool ProcessAliasOptionsArgs(lldb::CommandObjectSP &cmd_obj_sp,
 
   if (!options_string.empty()) {
     if (cmd_obj_sp->WantsRawCommandString())
-      option_arg_vector->emplace_back("<argument>", -1, options_string);
+      option_arg_vector->emplace_back(CommandInterpreter::g_argument, 
+                                      -1, options_string);
     else {
       for (auto &entry : args.entries()) {
         if (!entry.ref().empty())
-          option_arg_vector->emplace_back(std::string("<argument>"), -1,
+          option_arg_vector->emplace_back(std::string(CommandInterpreter::g_argument), -1,
                                           std::string(entry.ref()));
       }
     }
@@ -153,11 +154,12 @@ void CommandAlias::GetAliasExpansion(StreamString &help_string) const {
 
   for (const auto &opt_entry : *options) {
     std::tie(opt, std::ignore, value) = opt_entry;
-    if (opt == "<argument>") {
+    if (opt == CommandInterpreter::g_argument) {
       help_string.Printf(" %s", value.c_str());
     } else {
       help_string.Printf(" %s", opt.c_str());
-      if ((value != "<no-argument>") && (value != "<need-argument")) {
+      if ((value != CommandInterpreter::g_no_argument) 
+           && (value != CommandInterpreter::g_need_argument)) {
         help_string.Printf(" %s", value.c_str());
       }
     }
@@ -178,7 +180,7 @@ bool CommandAlias::IsDashDashCommand() {
 
   for (const auto &opt_entry : *GetOptionArguments()) {
     std::tie(opt, std::ignore, value) = opt_entry;
-    if (opt == "<argument>" && !value.empty() &&
+    if (opt == CommandInterpreter::g_argument && !value.empty() &&
         llvm::StringRef(value).endswith("--")) {
       m_is_dashdash_alias = eLazyBoolYes;
       break;
@@ -206,6 +208,8 @@ 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(),

lldb/source/Interpreter/CommandInterpreter.cpp

@@ -105,6 +105,11 @@ static constexpr const char *InitFileWarning =
     "and\n"
     "accept the security risk.";
 
+const char *CommandInterpreter::g_no_argument = "<no-argument>";
+const char *CommandInterpreter::g_need_argument = "<need-argument>";
+const char *CommandInterpreter::g_argument = "<argument>";
+
+
 #define LLDB_PROPERTIES_interpreter
 #include "InterpreterProperties.inc"
 
@@ -1634,7 +1639,7 @@ CommandObject *CommandInterpreter::BuildAliasResult(
   std::string value;
   for (const auto &entry : *option_arg_vector) {
     std::tie(option, value_type, value) = entry;
-    if (option == "<argument>") {
+    if (option == g_argument) {
       result_str.Printf(" %s", value.c_str());
       continue;
     }
@@ -1656,11 +1661,33 @@ CommandObject *CommandInterpreter::BuildAliasResult(
                                    index);
       return nullptr;
     } else {
-      size_t strpos = raw_input_string.find(cmd_args.GetArgumentAtIndex(index));
+      const Args::ArgEntry &entry = cmd_args[index];
-      if (strpos != std::string::npos)
+      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;  
+        }
         raw_input_string = raw_input_string.erase(
-            strpos, strlen(cmd_args.GetArgumentAtIndex(index)));
+            strpos - start_fudge, 
-      result_str.Printf("%s", cmd_args.GetArgumentAtIndex(index));
+            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);
     }
   }
 
@@ -1911,13 +1938,6 @@ 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
@@ -1935,6 +1955,20 @@ 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".
@@ -2163,7 +2197,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 == "<argument>") {
+      if (option == g_argument) {
         if (!wants_raw_input || (value != "--")) {
           // Since we inserted this above, make sure we don't insert it twice
           new_args.AppendArgument(value);
@@ -2174,7 +2208,7 @@ void CommandInterpreter::BuildAliasCommandArgs(CommandObject *alias_cmd_obj,
       if (value_type != OptionParser::eOptionalArgument)
         new_args.AppendArgument(option);
 
-      if (value == "<no-argument>")
+      if (value == g_no_argument)
         continue;
 
       int index = GetOptionArgumentPosition(value.c_str());

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().ref().front() == '`') {
+      if (!entry.value().ref().empty() && entry.value().GetQuoteChar() == '`') {
         cmd_args.ReplaceArgumentAtIndex(
             entry.index(),
             m_interpreter.ProcessEmbeddedScriptCommands(entry.value().c_str()));

lldb/source/Interpreter/Options.cpp

@@ -1002,37 +1002,50 @@ 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()) {
-      auto tmp_arg = args_copy[idx].ref();
+      llvm::StringRef 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 ((long_options[long_options_index].definition->option_has_arg !=
+    if ((option_to_insert != CommandInterpreter::g_no_argument) &&
-         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(option_to_insert);
-        size_t pos = input_line.find(std::string(tmp_arg));
         if (pos != std::string::npos)
-          input_line.erase(pos, tmp_arg.size());
+          input_line.erase(pos, option_to_insert.size());
       }
       args_copy.DeleteArgumentAtIndex(idx);
     }

lldb/source/Utility/Args.cpp

@@ -215,7 +215,12 @@ 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();

lldb/test/API/commands/command/backticks/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/commands/command/backticks/TestBackticksInAlias.py

@@ -0,0 +1,32 @@
+"""
+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")
+
+        

lldb/test/API/commands/command/backticks/main.c

@@ -0,0 +1,5 @@
+int main (int argc, char const *argv[])
+{
+  return argc; // break here
+}
+