On Fri, Apr 6, 2018 at 9:48 AM, Adrian Prantl <apra...@apple.com> wrote:
>> On Apr 6, 2018, at 9:32 AM, Davide Italiano <dccitali...@gmail.com> wrote:
>> On Fri, Apr 6, 2018 at 1:02 AM, Jan Kratochvil via Phabricator
>> <revi...@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
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.


lldb-commits mailing list

Reply via email to