DavidSpickett added inline comments.
================ Comment at: lldb/include/lldb/Target/MemoryTagMap.h:29 + /// Non-null pointer to a memory tag manager. + MemoryTagMap(const MemoryTagManager *manager); + ---------------- JDevlieghere wrote: > If the pointer should be non-null, then should this take a reference instead? I would like to do that but in a later patch I have: ``` static llvm::Optional<MemoryTagMap> GetMemoryTags(lldb::addr_t addr, size_t length, ExecutionContextScope *exe_scope) { ``` Can't have references in an optional. However looking again, I think I could check `.empty()` instead of using an Optional. I'll try that. ================ Comment at: lldb/include/lldb/Target/MemoryTagMap.h:46 + + bool empty() const; + ---------------- JDevlieghere wrote: > I'm on the fence about this one. I personally prefer `empty()` for > consistency with llvm and the standard library, but technically this should > be `Empty()` to match the rest of LLDB. I see 7 `Empty` and 3 `empty`so I'll go with the former. Not going to put it through any standard algorithms in any case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112825/new/ https://reviews.llvm.org/D112825 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits