[Lldb-commits] [lldb] [lldb] Move GetSelectedTarget from Debugger to CommandInterpreter (NFC) (PR #197805)

[via lldb-commits]

jimingham wrote:

So the fact that in all the correct uses of GetSelectedTarget you were replacing:

interpreter.GetDebugger().GetSelectedTarget()

with

interpreter.GetSelectedTarget()

is a good sign this was in the wrong place and it belonged in the CommandInterpreter.  

Also, that most of the places where the code was just using GetDebugger().GetSelectedTarget() were arguably wrong is another good sign this shouldn't have been on the Debugger.

I think it would be interesting to keep the list of places you had to change here for further review.  They seem to fall into three classes:

MPC code/GUI IOHandler - Grouped because I don't really understand how either of them work...

HandleCompletion - Using the Selected Target is formally wrong here, though it probably doesn't matter.  If the selected Target was Target A, and Target B hit a breakpoint and for some odd reason the breakpoint callback wanted to do SBCommandInterpreter::HandleCompletion() it would get completion in the wrong context.  But it's not entirely clear why you would ever want to do this, and I'm not so sure how you would get the right context to HandleCompletion.  So while formally wrong we probably have to live with this.

A bunch of CommandObject subclasses were still calling GetSelectedTarget but so far as I can tell they really should be using CommandObject::GetTarget instead.  Any command that might get used in any of our callbacks (breakpoint commands, stop-hooks, etc) needs to get the target set in the command object and not the CommandInterpreter's Selected Target.  It's probably worth fixing them since this change made them evident.  

I can't think of a way to enforce "CommandObject subclasses' DoExecute implementation cannot call CommandInterpreter::GetSelectedTarget but their HandleCompletion methods can" for instance.  So I think we have to enforce "use GetTarget not interpreter.GetSelectedTarget in DoExecute" by auditing.



https://github.com/llvm/llvm-project/pull/197805