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

Reply via email to