[Lldb-commits] [PATCH] D133045: Partial fix for handling backticks in commands and aliases

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, clayborg.
Herald added a subscriber: jeroen.dobbelaere.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

At present, backtick substitution is a "preprocessor" operation, which is not, I think, what people expect.  I'm pretty sure you didn't mean to run an expression when you specified a file name that happened to have backticks in it, etc.  I think it makes a lot more sense that the substitution should be done when it is encountered as a quote character.  There was, in fact, some code in CommandObjectParsed that attempted to treat it this way, but it was defeated by the fact that we unconditionally do PreprocessCommand on the command string before parsing.  It was also not quite right since it assumed the quote would still be present in the argument value, when in fact it was just in the quote member of the ArgEntry...

I fixed that, and changed the CommandInterpreter code to only preprocess the whole string for raw commands.  I think really that Raw commands should handle their own preprocessing, for instance I don't think doing backtick substitution in the expression passed to `expr` is really what people expect...

When I did that a bunch of tests failed because we were not correctly preserving the backticks around arguments when we parsed them into a temporary argv array and then back out again, and because we didn't take them into account when stripping elements out of the incoming command when handling command aliases.

I also added a test that  explicitly tests command aliases with embedded backticks.  The test has a succeeding case and a failing case.  The latter shows an extant bug this patch doesn't fix: the CommandAlias::Desugar doesn't correctly handle the case where you alias an alias and the second alias fills in slots of the first alias.  It just pastes the second aliases options after the first aliases and then tries to evaluate them, which ends up with unfilled slots.  As I say, that's an extant bug so the test for it fails now.  I'll fix that one next chance I have to mess with this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133045

Files:
  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
  lldb/test/API/commands/command/backticks/Makefile
  lldb/test/API/commands/command/backticks/TestBackticksInAlias.py
  lldb/test/API/commands/command/backticks/main.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133045.457063.patch
Type: text/x-patch
Size: 14191 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220831/461c26aa/attachment-0001.bin>