[Lldb-commits] [PATCH] D111136: [lldb] [DynamicRegisterInfo] Support iterating over Registers()

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.


================
Comment at: lldb/include/lldb/Target/DynamicRegisterInfo.h:82
+  llvm::iterator_range<reg_collection::iterator> Registers() {
+    return llvm::iterator_range<reg_collection::iterator>(m_regs);
+  }
----------------
labath wrote:
> mgorny wrote:
> > ted wrote:
> > > mgorny wrote:
> > > > labath wrote:
> > > > > Could this return const iterators? It seems we already have some non-const accessors, but I'd rather not propagate that..
> > > > It can't — we're using these iterators to augment register infos ;-).
> > > Maybe make a non-const protected, and a const public, so random plugin #37 can't modify register info?
> > That would work for me; however, we'd have to make `ABI` and `DynamicRegisterInfo` `friend`s then. @labath, what do you thik?
> Yeah, I forgot what motivated this change...
> 
> I don't think friending would work the way you imagined it (one would need some extra tricks) as friendship is not inherited by subclasses. That wouldn't be the end of the world, but I am hoping we can come with a better solution.
> 
> So, currently we have this `RemoteRegisterInfo` struct as a kind of a DynamicRegisterInfo precursor. What if the ABI classes operated on that instead ? Besides solving the visibility issues (this data isn't live yet, so we're free to modify it as we see fit), I'm hoping it would also make things simpler as the data hasn't been converted to the C format yet. We could move RemoteRegisterInfo to the DynamicRegisterInfo class (and rename it to something else, obviously) so that it's visible to everyone involved.
> 
> Currently Finalization happens very close to the point where we convert the remote format, so I'm hoping this wouldn't be too hard to implement. The presence of the `HardcodeARMRegisters` calls complicates things a bit. The main difference would be that these hardcoded registers wouldn't go through the augmentation process (without additional refactoring), but from the looks of it, they don't actually need any augmentation, so we could just let them float.
> 
> WDYT?
Sure, I'll see if this can be done cleanly.


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

https://reviews.llvm.org/D111136