labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
I like this a lot. LGTM with some small fixes inline.
================
Comment at: lldb/source/Host/common/NativeRegisterContext.cpp:428-434
+ static const uint32_t k_expedited_registers[] = {
+ LLDB_REGNUM_GENERIC_PC, LLDB_REGNUM_GENERIC_SP, LLDB_REGNUM_GENERIC_FP,
+ LLDB_REGNUM_GENERIC_RA, LLDB_INVALID_REGNUM};
+
+ std::vector<uint32_t> expedited_reg_nums;
+ for (const uint32_t *generic_reg_p = k_expedited_registers;
+ *generic_reg_p != LLDB_INVALID_REGNUM; ++generic_reg_p) {
----------------
Remove LLDB_INVALID_REGNUM from the list. Then iterate as `for(uint32_t
gen_reg: k_expedited_registers)`.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:513-515
+ if (expedited_regs.empty())
+ return llvm::make_error<llvm::StringError>("failed to get registers",
+ llvm::inconvertibleErrorCode());
----------------
Let's change the result type to `Optional<Object>` and `return None` here --
now that we support customizing the expedited registers, I think it's
reasonable to allow a register context to say it does not want to expedite any
registers. (The current for of the code already kind of supports that, but the
use of Expected makes it sound like that is an erroneous state.)
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:798
- for (const uint32_t *reg_num_p = reg_set_p->registers;
- *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
+ if (!expedited_regs.empty()) {
+ for (auto ®_num : expedited_regs) {
----------------
I don't think this if is needed now -- we could remove it and save one level of
indentation.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82853/new/
https://reviews.llvm.org/D82853
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
