[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

Adding an SB API to set the target's search paths seems fine to me.  I think in theory the totality of LLDB functionality should be available through the SB API's, (a) because it is always awkward to fall back on HandleCommand and (b) because at some point we really should make the command-line be a fairly simple client of the SB API's as that would greatly reduce our testing surface.  We're only going to get there one bit at a time.

But this seems the wrong way to do it.  You are setting a property on an SBTarget so this should be a method on the SBTarget.  The selected target is really not reliable.  For instance, having this only available for the selected target would make this impossible to use in a breakpoint command, since you could have two running targets, target A could be selected but target B could hit a breakpoint.  We won't select B till we've decided that B is actually going to stop - which you don't know when you are executing breakpoint commands.  So API like this should really not rely on the selected target.  BTW, I see this is right below SBDebugger::SetCurrentPlatformSDKRoot, but I don't approve of that one either...

Also, if you're going to the trouble of adding a "AddSharedObjectPath, you should also add ListSharedObjectPaths, and that will make it easier to write a test for this command.


Repository:
  rL LLVM

https://reviews.llvm.org/D47302