[Lldb-commits] [PATCH] D52375: [WIP] Support multiple compile units per OSO entry in SymbolFileDWARFDebugMap

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:430
     const size_t num_ranges =
-        die->GetAttributeAddressRanges(dwarf, this, ranges, false);
+        die->GetAttributeAddressRanges(dwarf, this, ranges, true);
     if (num_ranges > 0) {
----------------
Please give this bool a name, either by assigning it to a named variable or by prefixing it with a comment like `/* check_hi_lo_pc */`. I think the LLDB code base prefer the former?


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:786
+        assert(debug_map == m_debug_map_symfile &&
+               "It's a nested instance derived from SymbolFile, "
+               "NOT SymbolFileDWARF");
----------------
What is "it" here?


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:800
+            // FIXME: This must never happen!
+            assert(false && "Need more info to find the correct CU");
+            cu_sp = debug_map->GetCompileUnit(this);
----------------
You can use `llvm_unreachable` here.


================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1338
+                                                dw_offset_t cu_offset) {
+  // TODO: Can we assume partial order in offsets and bsearch?
+  const uint32_t cu_count = GetNumCompileUnits();
----------------
Yes, otherwise we have invalid DWARF, which I don't think it's worth supporting something like that here.


https://reviews.llvm.org/D52375