[Lldb-commits] [PATCH] D87442: [lldb] Show flags for memory regions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Woa, back up. I though you were just going to add the one flag you need right now... :(

I was going for the showing flags as a feature of the "memory region" command then later adding the memory tagging flag to that.
I see your point though and yeah I don't need all the flags to unblock mte.

With the perspective I was coming from, adding a set of getter/setter for 20ish flags wasn't an appealing idea:

  bool m_memory_tagged;
  OptionalBool GetMemoryTagged() const { return m_memory_tagged; }
  void SetMemoryTagged(bool v) { m_memory_tagged = v; }
  <x20>

If it's just mt then I just add another set, can always merge it with a flags (plural) interface later if we accumulate more.

So if it makes sense to you, I will:

- add only "mt" flag, with it's own getter/setter
- keep the protocol changes (but only recognise the 1 flag)
- keep the extra testing, use of expected etc. where it still applies
- add tests to run on a memory tagging enabled kernel with qemu

Then we can at least agree in principle, even if this doesn't land until the new tagging commands have also been reviewed. (which is fine by me, but I don't have them ready yet)

Thanks for all your comments so far!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87442