https://github.com/labath commented:
I think something like this makes sense, but if we're doing this, I think we should do it properly and actually remove the DWARFExpression->SymbolFileDWARF dependency. We can't do that if `DWARFUnitInterface` is defined in the symbol file plugin, nor while it contains methods like `GetSymbolFileDWARF()`. Moving the interface definition elsewhere is easy. The second part requires some thought. It looks like the main use for that method is to provide vendor extensibility for dwarf parsing, so I think we should expose *that* as an interface (i.e., have methods like ParseVendorDWARFOpcodeSize and GetVendorDWARFOpcodeSize) instead of the raw DWARF symbol file. For this reason, I also think the interface shouldn't be called DWARFUnitInterface, but rather something centred on DWARFExpression and the things it needs from its users. For lack of imagination, I'm going to suggest DWARFExpression::Delegate (because we already use that term in some places), but maybe we'll come up with something more specific in the process. (This means it may not make sense to DWARFUnit to inherit from DWARFExpression::Delegate, but that's okay -- we can always use composition instead) I also want to look at each function in that interface and see if it's really necessary. Two examples: - GetLocationTable is only used from ParseDWARFLocationList and *that* is only used from DWARF plugin code. Instead of adding this to the interface, I think we should move ParseDWARFLocationList into the DWARF plugin. (This can/should be a separate patch) - I'm not sure that `GetAddressByteSize` is necessary since the DataExtractor member (`m_data`) already has a method with the same name (and it's even used in some places). Before adding that to the interface, I'd like to see if we can make everything use the value in m_data instead. https://github.com/llvm/llvm-project/pull/131645 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
