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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PAT... David Spickett via Phabricator via lldb-commits
    • [Lldb-commits]... Muhammad Omair Javaid via Phabricator via lldb-commits
    • [Lldb-commits]... David Spickett via Phabricator via lldb-commits

Reply via email to