> -----Original Message----- > From: llvm-commits [mailto:llvm-commits-boun...@lists.llvm.org] On Behalf > Of Jan Kratochvil via llvm-commits > Sent: Friday, April 06, 2018 5:29 PM > To: Greg Clayton > Cc: reviews+d45170+public+e75ed5903a857...@reviews.llvm.org; Jason > Molenda; lldb-commits@lists.llvm.org; llvm-commits; Davide Italiano > Subject: Re: [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in > preparation for adding DWARFTypeUnit > > 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
I have to say I am a little surprised by this hierarchy. I would have thought type units and partial units would be more alike. Type units are imported by signature not reference but otherwise I'd think they would have a lot in common: lumps of DWARF that aren't their own compilation unit. --paulr > > 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 > _______________________________________________ > llvm-commits mailing list > llvm-comm...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
