[Lldb-commits] [PATCH] D105179: [lldb][AArch64] Add tag repeat and unpack to memory tag manager

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

This looks good. I just made some cosmetic comments.



================
Comment at: lldb/include/lldb/Target/MemoryTagManager.h:79
+  //
+  // Set granules to 0 to skip checking the number of tags found.
   virtual llvm::Expected<std::vector<lldb::addr_t>>
----------------
Do you think it will be better if we default granules to 0 then granules will become optional?


================
Comment at: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp:61
+  MemoryTagManagerAArch64MTE manager;
+
+  llvm::Expected<std::vector<uint8_t>> invalid_tag_err =
----------------
May be add a comment here as you have done for all the tests above and below this one.


================
Comment at: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp:153
+
+static void
+test_repeating_tags(const std::vector<lldb::addr_t> &tags,
----------------
May be add a comment explaining this helper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105179