[Lldb-commits] [PATCH] D69014: [LLDB] bugfix: command script add -f doesn't work for some callables

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

IIUC, the only external change in this patch is that before if you implemented your Python command using a class with an `__call__` method, it would have to be the signature that took exe_ctx.   Since this is now switching off of the number of arguments (less self) you could make an `__call__` form that didn't take the extra argument.  I certainly want to discourage that, since the form without the exe_ctx will do the wrong thing in some contexts (e.g. if the command is used in breakpoint callbacks or stop-hooks).  It looks like that would be easy to prevent, you know that you've found __call__ right above where you check.  So I think it would be better to explicitly disallow __call__forms that don't take the exe_ctx.

This is different from the extra_args cases in the breakpoint callbacks and thread plans and so forth.  In those cases, if you don't need extra_args, there's no good reason why you should have to have them.  But in the case of Python commands, it's actually more correct to get the state you are operating on from the exe_ctx.  Otherwise you have to fall back on the currently selected process/thread/frame/etc, and there are contexts (like when you have multiple targets all hitting breakpoints simultaneously) where it does not make sense to have the currently selected state track what process/thread/frame are pertinent to the operation of a breakpoint command or stop hook.  So I do want to gently discourage this form of command.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69014