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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 28 12:20:08 PDT 2019


labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Awesome.

In D66863#1649436 <https://reviews.llvm.org/D66863#1649436>, @jingham wrote:

> 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.


This sounds like a good idea to me.



================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:2615-2617
+            "Set the load addresses for "
+            "one or more sections in a "
+            "target module.",
----------------
Another quirk of clang-format is that it will not re-merge strings that it has split previously. If you manually join these three strings into one, and then re-run clang-format, you'll probably end up with something slightly nicer...


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66863





More information about the lldb-commits mailing list