[PATCH] D39854: [DWARFv5] Support FORM_strp in .debug_line.dwo

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:213-217
+static void
+buildLineToUnitMap(DWARFContext::cu_iterator_range CUs,
+                   DWARFContext::tu_section_iterator_range TUSections,
+                   LineToUnitMap &LineToUnit) {
+  LineToUnit.clear();
----------------
Might make more sense for this function to return the map by value, rather than take it by reference, clear it, and populate it?


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:336
   uint8_t savedAddressByteSize = 0;
+  // FIXME: This seems sketchy.
+  for (const auto &CU : compile_units()) {
----------------
Probably be more specific/descriptive.

This also appears to be a regression - before your patch this code did the right thing (used the corresponding CU's address byte size for each CU) for non-DWO CUs, though it was probably wrong for DWO CUs by using this saved thing. But with your patch it now does the same sketchy thing for DWO and non-DWO CUs?


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:337-340
+  for (const auto &CU : compile_units()) {
+    savedAddressByteSize = CU->getAddressByteSize();
+    break;
+  }
----------------
A loop that only runs once seems a bit odd, though I realize it's pretty compact.


https://reviews.llvm.org/D39854