[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:24
+/// set,
+///   the DIERef references the main, dwo or .o file.
 /// - section: identifies the section of the debug info entry in the given file:
----------------
I don't understand this sentence.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:32
   enum Section : uint8_t { DebugInfo, DebugTypes };
-
-  DIERef(std::optional<uint32_t> dwo_num, Section section,
+  // Making constructors protected to limit where DIERef is used directly.
+  DIERef(std::optional<uint32_t> file_index, Section section,
----------------
they're not actually protected


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp:75
+  if (ref)
+    return DIERef(*GetDWARF(), *ref).get_id();
+
----------------
Is this the only call site of the `DIERef(SymbolFileDWARF&)` constructor?

If so, and if we make it such that `DWARFBaseDIE::GetDIERef` returns the fully filled in DIERef, then this function can just call get_id() on the result, and we can delete that constructor.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1682
 SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
-  if (die_ref.dwo_num()) {
-    SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fffffff
-                                 ? m_dwp_symfile.get()
-                                 : this->DebugInfo()
-                                       .GetUnitAtIndex(*die_ref.dwo_num())
-                                       ->GetDwoSymbolFile();
-    return dwarf->DebugInfo().GetDIE(die_ref);
-  }
-
-  return DebugInfo().GetDIE(die_ref);
+  return GetDIE(die_ref.get_id());
 }
----------------
clayborg wrote:
> Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be the one source of truth when finding a DIE. We could make "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then have "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and then call "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.
+1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138618/new/

https://reviews.llvm.org/D138618