omjavaid marked 3 inline comments as done.
omjavaid added inline comments.

================
Comment at: 
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp:46
+    : lldb_private::RegisterContext(thread, 0) {
+  m_register_info_up = std::move(register_info);
 
----------------
labath wrote:
> move this to the initializer list
ACK.


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp:85
+    gpr_w22, gpr_w23,  gpr_w24, gpr_w25, gpr_w26, gpr_w27, gpr_w28,
+    LLDB_INVALID_REGNUM // register sets need to end with this flag
+};
----------------
labath wrote:
> I'd probably just delete this comment (or merge it with the leading comment 
> above the array definition), and then let clang-format lay this out 
> normally...
Let me check what clang-format emits.


================
Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h:19
 public:
+  enum { ARM64GPR = 0, ARM64FPR };
+
----------------
labath wrote:
> Why these names? I think [GF]PRegSet would be better for two reasons:
> - the same names with the same purpose already exist in 
> `NativeRegisterContextNetBSD_x86_64.h`
> - it seems like a better way to differentiate from the [GF]PR structs below 
> than adding a redundant ARM64 prefix.
Indeed ARM64 is redundant now that we have these enums in 
RegisterInfosPOSIX_arm64 . I will fix this in updated revision.


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

https://reviews.llvm.org/D80105



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to