jankratochvil added inline comments.
================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216 + DWARFCompileUnitData *m_data; + ---------------- jankratochvil wrote: > clayborg wrote: > > jankratochvil wrote: > > > clayborg wrote: > > > > Is there a reason this is a member variable that I am not seeing? Seems > > > > we could have this class inherit from DWARFCompileUnitData. I am > > > > guessing this will be needed for a future patch? > > > Yes, future patch D40474 contains a new constructor so multiple > > > `DWARFCompileUnit` then point to single `DWARFCompileUnitData`. Sure > > > that happens only in the case ot `DW_TAG_partial_unit` (one > > > `DWARFCompileUnit` is read from a file while other `DWARFCompileUnit` are > > > remapped instances with unique offset as used from units which did use > > > `DW_TAG_imported_unit` for them). > > > `DWARFCompileUnit(DWARFCompileUnitData *data, DWARFCompileUnit *main_cu);` > > > > > We should just have an instance of this in this class and add a virtual > > function to retrieve it. Then we make a DWARFPartialUnit that subclasses > > this and has its own extra member variable and can use either one when it > > makes sense. Not a fan of just having a dangling pointer with no clear > > ownership. There is no "delete m_data" anywhere? > The `DWARFCompileUnitData *m_data` content is being held in: > `std::forward_list<DWARFCompileUnitData> > SymbolFileDWARF::m_compile_unit_data_list` as I hope/believe a > `DWARFCompileUnit` is never deleted until whole `SymbolFileDWARF` is deleted. > I did not use `std::shared_ptr<DWARFCompileUnitData> > DWARFCompileUnit::m_data` as `shared_ptr` is both memory and > lock-instruction-performance expensive. > > Then we make a DWARFPartialUnit that subclasses this We cannot because DWARFCompileUnit is constructed by `DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded`->`DWARFCompileUnit::Extract` and it currently does not know whether it is `DW_TAG_compile_unit` or `DW_TAG_partial_unit`. But then `DWARFCompileUnit::Extract` could read even the very first DIE and save it for later use. Then `DWARFCompileUnit::ExtractDIEsIfNeeded` would not need the `bool cu_die_only` parameter and altogether I could also drop my D40471. Do you agree? (This whole DWARF units parsing across the file is not too efficient anyway, there should be some index in use; and I would prefer the DWARF-5 one over the Apple one.) https://reviews.llvm.org/D40466 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
