[Lldb-commits] [PATCH] D96194: Defer the decision whether to use the CU or TU index until after reading the unit header.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added a comment.

I'm going to go ahead and commit this given that I have just made the last suggested modification and the patch has already been up for review without further comments for a long while (sorry for the late replies, work keeps getting in the way of work).



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:808-809
 
+  if (cu_index && (header.m_unit_type == llvm::dwarf::DW_UT_compile ||
+                   header.m_unit_type == llvm::dwarf::DW_UT_split_compile)) {
+    header.m_index_entry = cu_index->getFromOffset(header.m_offset);
----------------
labath wrote:
> jgorbe wrote:
> > labath wrote:
> > > I guess this could be `header.IsTypeUnit()` (and `!header.IsTypeUnit())`)...
> > But `!header.IsTypeUnit` would also treat DW_UT_partial and DW_UT_skeleton as compile units, right?
> That's true, but can either of those units legitimately appear in a dwp file?
> Even if they do appear for whatever reason, it wouldn't make any sense to use them without an index entry, and it would be (somewhat) more reasonable to put them in the cu index.
> 
> I took this idea from the equivalent llvm code: <https://github.com/llvm/llvm-project/blob/main/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp#L81>
Good point. Changed.


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

https://reviews.llvm.org/D96194