omjavaid added a comment. I think this approach is a lot better given that the process of tagged ranged calculation deserved a separation and may be more explanation too. I guess somewhere in the comments of same function you can explain interaction between memory regions and tagged ranges. It will become a bit more complicated for a reader who have no knowledge about memory tag range and disjoint region.
================ Comment at: lldb/include/lldb/Target/MemoryTagManager.h:66 + // If so, return a modified range. This will be expanded to be + // granule aligned and have an untagged base address. + virtual llvm::Expected<TagRange> MakeTaggedRange( ---------------- I couldnt understand this point in comment "have an untagged base address" ================ Comment at: lldb/source/Commands/CommandObjectMemoryTag.cpp:70 Process *process = m_exe_ctx.GetProcessPtr(); llvm::Expected<const MemoryTagManager *> tag_manager_or_err = + process->GetMemoryTagManager(); ---------------- If we also pull out the SupportsMemoryTagging out of GetMemoryTagManager we can directly return a tag manager pointer here. Also SupportsMemoryTagging may also run once for the life time of a process? We can store this information is a flag or something. ================ Comment at: lldb/source/Commands/CommandObjectMemoryTag.cpp:71 llvm::Expected<const MemoryTagManager *> tag_manager_or_err = - process->GetMemoryTagManager(start_addr, end_addr); + process->GetMemoryTagManager(); ---------------- should we process pointer validity check before this call? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105630/new/ https://reviews.llvm.org/D105630 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits