[PATCH] D86962: [LLDB] Move NativeRegisterContextLinux/RegisterContextPOSIX*_arm to RegisterInfoAndSetInterface

[Pavel Labath via Phabricator via llvm-commits]

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

This is great, thanks for doing this. Just a couple of minor comments inline.



================
Comment at: lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp:167
     case llvm::Triple::aarch64:
       break;
     case llvm::Triple::arm:
----------------
delete this break.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp:61-62
                                  new RegisterInfoPOSIX_arm(target_arch)) {
   switch (target_arch.GetMachine()) {
   case llvm::Triple::arm:
+    ::memset(&m_fpr, 0, sizeof(m_fpr));
----------------
Actually, I'd just change this to `assert(target_arch.GetMachine() == llvm::Triple::arm)`


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm.cpp:93-95
+static_assert(((sizeof g_gpr_regnums_arm / sizeof g_gpr_regnums_arm[0]) - 1) ==
+                  k_num_gpr_registers,
+              "g_gpr_regnums_arm has wrong number of register infos");
----------------
You can skip this if you want to keep things consistent (or put it in a separate patch, etc), but I just realized that this is completely unnecessary. Instead of the assert, we could just make the assert expression _be_ the definition of the constant (`k_num_gpr_registers = ((sizeof g_gpr_regnums_arm / sizeof g_gpr_regnums_arm[0]) - 1)`). Given that `llvm::array_lengthof` is constexpr, it might even work to make this `llvm::array_lengthof(g_gpr_regnums_arm)-1`.


================
Comment at: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp:84-85
       switch (arch.GetMachine()) {
       case llvm::Triple::aarch64:
         break;
       case llvm::Triple::ppc:
----------------
I think you should either keep both aarch64 and arm cases, or delete both of them. I'm fine with either.


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

https://reviews.llvm.org/D86962