Many things that were in DWARFCompileUnit actually need to be in DWARFUnit. This patch moves all DWARFUnit specific things over into DWARFUnit and fixes the layering. This is in preparation for adding DWARFTypeUnit for the .debug_types patch.
Details
- Reviewers
labath davide aprantl jasonmolenda - Commits
- rG118bcd9ce247: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit
rLLDB329305: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit
rL329305: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit
Diff Detail
- Repository
- rL LLVM
Event Timeline
Thanks Greg. This is a very large patch, but it seems mostly churn. I'll try to find the time to review it carefully tomorrow.
Yes this is just moving all ivars from DWARFCompileUnit to DWARFUnit, moving all functions that used those accessors to DWARFUnit and remove the indirection through DWARFCompileUnit that was in DWARFUnit using:
virtual DWARFCompileUnit &Data() = 0;
No real functional change, just fixing layering and moving code. Also a renaming from GetCompileUnitDIEOnly to GetUnitDIEOnly.
I have only one general question and otherwise I'm fine with this: Does this bring LLDB's DWARF parser closer to the structure of LLVM's? We still want to long-term replace LLDB's DWARF parser with LLVM's so every refactoring should aim at making their interfaces more similar.
I disagree with this patch as DWARFUnit was a lightweight wrapper for DWARFPartialUnit. Now I will have to create some new lightweight superclass like DWARFAbstractUnit.
My patch prepared it for:
DWARFUnit->DWARFCompileUnit DWARFUnit->DWARFPartialUnit
And I planned the type units should be implemented like:
DWARFUnit->DWARFSomeNameUnit->DWARFCompileUnit DWARFUnit->DWARFSomeNameUnit->DWARFTypeUnit DWARFUnit->DWARFPartialUnit
This patch just reused + changed my abstraction for a completely different purpose and I will have to reimplement it again under a different name. Or what do you suggest?
I have reverted this commit after approval by Davide Italiano and Adrian Prantl at:
https://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20180402/040176.html
The revert:
git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@329423 91177308-0d34-0410-b5e6-96231b3b80d8 GIT commit a4b4ca0aef594f71e44478c2f16ca9fd53938ec6