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

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 6 09:48:03 PDT 2018



> On Apr 6, 2018, at 9:32 AM, Davide Italiano <dccitaliano at gmail.com> wrote:
> 
> On Fri, Apr 6, 2018 at 1:02 AM, Jan Kratochvil via Phabricator
> <reviews at reviews.llvm.org> wrote:
>> jankratochvil added a comment.
>> 
>> 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?
>> 
> 
> As there's some disagreement on how to proceed forward, we can
> probably revert this for now and start a discussion.
> You can probably do it yourself.

Just please make sure that whatever variant we end up deciding upon, the interface must become more like llvm's DWARF* classes, even if that means changing the llvm hierarchy to become more like LLDB's if need be. The end goal should be to make LLVM's DWARF classes good enough to be used in LLDB so we don't need to maintain two implementations.

-- adrian


More information about the llvm-commits mailing list