[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am trying to get rid of the friendship declaration and the forwarding of protected methods. I have no issue with making `GetCompileUnitAtIndex` virtual -- that is pretty much a necessity for this feature (*). However, `SetCompileUnitAtIndex` seems like an internal implementation detail of a SymbolFile instance (that's why it's protected), and it would be better if the new class didn't have to touch it. I wouldn't call the new class the "real" symbol file. I don't see it as any more real as any other subclass. The way I'd look at it is that the new class just contains some helpful implementation that can be useful to a specific symbol file class. An individual symbol file class may choose to use it, but it could also implement the pure SymbolFile interface is a completely different way, if it knows how.

(*) Well, I can also imagine an implementation where the on-demand logic lives inside the SymbolFile class (in public non-virtual methods), and the individual subclasses are implementing just the protected virtual (and maybe abstract) interface that is called from the public functions. That would be in line with the non-virtual interface <https://en.wikipedia.org/wiki/Non-virtual_interface_pattern> design pattern, and would avoid having the `GetBackingFile` method, but I'm not sure if it would be cleaner overall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121631