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

[Davide Italiano via lldb-commits]

On Fri, Apr 6, 2018 at 9:48 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
>
>> 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

Yes, I definitely agree. It's clear this needs more discussion, so I
don't think it's reasonable if we revert this for now and
reconsider.
I'll also take a look at the interfaces in LLVM to get a better sense
of what should be done. If nobody does this, I'll probably get to the
revert by the end of the day.

Best,

--
Davide