[Lldb-commits] [PATCH] D63399: DWARF: Make DIERefs always valid

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am concerned that our mapping from DIERef to lldb::user_id_t won't work for all cases now that we are/have expanded the DIERef class (including as we add the DWO field). I voiced this concern in https://reviews.llvm.org/D63428. Let me know what you think.



================
Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:59
 
   DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *cu_offset);
   if (!cu)
----------------
Shouldn't we be using the section from the DIERef in "entry" instead of hard coding to "DIERef::Section::DebugInfo" here? If so we need a test to cover this case so we don't regress


================
Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:70
   if (llvm::Optional<uint64_t> die_offset = entry.getDIEUnitOffset())
     return DIERef(DIERef::Section::DebugInfo, *cu_offset, die_bias + *die_offset);
 
----------------
Shouldn't we be using the section from the DIERef in "entry" instead of hard coding to "DIERef::Section::DebugInfo" here?



================
Comment at: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h:64
+    explicit operator DIERef() const {
+      return DIERef(DIERef::Section::DebugInfo, DW_INVALID_OFFSET, die_offset);
+    }
----------------
DIEInfo objects are only ever used in .debug_info?


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

https://reviews.llvm.org/D63399