[Lldb-commits] [PATCH] D66863: [lldb] Unify target checking in CommandObject

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.
Herald added a subscriber: JDevlieghere.

The whole command flags was a late addition, but we were using it for new commands and "when you touch it" kind of changes.  That seems to have stalled the conversion, so thanks for completing this!

As a minor style thing, prior to this change CommandObjectThreadStepUntil required a thread, but didn't require a process explicitly, because you can't have a thread without a process.  You also can't have a thread without a target since you can't have a process without a target.  But your change means that if I specify that I require a thread I now ALSO have to require a target explicitly or I will get an assert calling GetSelectedTarget.  That seems a little awkward to me (especially since you still don't have to require a process...)  It would be better if eCommandRequiresThread implied eCommandRequiresProcess & eCommandRequiresTarget.  That would keep these definitions less noisy.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66863