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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to