[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

[Jan Kratochvil via lldb-commits]

On Fri, 06 Apr 2018 22:52:05 +0200, Greg Clayton wrote:
> Switching over to the LLVM parser will require some detailed work and will
> take some time.

So should I continue pushing the DWZ patchset even before the reuse of LLVM
DWARFUnit happens?


> That being said, I am confused as to why this was reverted. The code I added
> mirrors the LLVM code a bit better, and yes it will require some reworking
> of your patches.

Planned DWARFPartialUnit needs the inheritance as it was.  As you have moved
most of the data fields back to DWARFUnit I will need to create some new
superclass DWARFSomeNameUnit and move back the data fields from DWARFUnit to
DWARFSomeNameUnit to get:
	DWARFUnit->DWARFSomeNameUnit->DWARFCompileUnit
	DWARFUnit->DWARFSomeNameUnit->DWARFTypeUnit
	DWARFUnit->DWARFPartialUnit

If you expected my DWARFUnit will serve as a superclass for
DWARFCompileUnit+DWARFTypeUnit then you should not have approved it the way I
wrote it as my goal was DWARFPartialUnit which has very different inheritance
requirements than DWARFTypeUnit.


> The DWARFUnit having an accessor to give out
> a DWARFCompileUnit was really confusing and not the right layering.

This is how DWARFPartialUnit works, it is only a DWARFCompileUnit remapped to
new offset.  I do not see how to implement it transparently without the
accessor (and without needlessly copying all the data fields many times into
each DWARFPartialUnit instance).


> So I fixed the layering. I need to submit .debug_types patches and that
> patch was needed for this and now I am back to square one.

I will sure deal with your reverting of my revert.  I just do not like the
words "fixes the layering" as it rather "changes the layering" for different
purposes than my DWARFUnit was implemented for.


Jan