[Lldb-commits] [PATCH] D149379: [lldb] Add tests for command removal

Mon May 15 14:51:07 PDT 2023

jingham added a comment.

In D149379#4331059 <https://reviews.llvm.org/D149379#4331059>, @wallace wrote:

> The alias still works because it still holds a reference to it. I could add that as a test

The alias works but the original command is gone?  Interesting.  We should definitely test that that works correctly.  We should also test what happens when a regex command dispatches to a command that has been deleted.

Also, should the "delete" operation fail or return an error if the actual command is still made available because of an alias.  If your intention was to remove a built-in command (still not sure why that is an operation that makes sense, but that aside...) then you haven't if you can still get to it through an alias.  Should we tell the user that?

TTTT, removing user commands is cosmetic if you don't remove `script` as well, since there's pretty much nothing that can be done on the command line that can't be done with scripts.  So I don't have a sense for how strict you are trying to be.  But if I can defeat the removal of a command by putting an alias in my .lldbinit, you are being very not strict...

================
Comment at: lldb/include/lldb/API/SBCommandInterpreter.h:316

+  /// Remove a command if it is removable (python or regex command). If \b force
+  /// is provided, the command is removed regardless of its removable status.
----------------
You should probably say explicitly what the "removable status" of a built-in command is.  You (correctly) have to explicitly pass force=true for that to work but it would be good to state that explicitly.  We should also emphasize here there isn't any way to get a built-in command back once you've deleted it.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149379/new/

https://reviews.llvm.org/D149379