aprantl added inline comments.
================ Comment at: packages/Python/lldbsuite/test/test_categories.py:27 'gmodules': 'Tests that can be run with -gmodules debug information', + 'dwarf_type_units' : 'Tests using the DWARF type units (-fdebug-types-section)', 'expression': 'Tests related to the expression parser', ---------------- clayborg wrote: > aprantl wrote: > > This is conflating two things (-fdebug-types-section and type units) DWARF5 > > doesn't have a debug_types section but it still offers type units. Clang > > doesn't implement this yet, but GCC might, I haven't checked. > So what is the suggestion here? I would mention type units in the comment rather than -fdebug-types-section. ================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:97 void DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded() { if (m_compile_units.empty()) { if (m_dwarf2Data != NULL) { ---------------- We probably should convert this to an early exit. ================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:106 offset = cu_sp->GetNextCompileUnitOffset(); } + ---------------- Perhaps add a comment explaining what is being done here? ================ Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:121 + break; + } + } ---------------- I find hard to follow. What do you think about: ``` while (debug_types_data.ValidOffset(offset)) { cu_sp = DWARFCompileUnit::Extract(m_dwarf2Data, debug_types_data, &offset, true))); if (!cu_sp) return; m_type_sig_to_cu_index[cu_sp->GetTypeSignature()] = m_compile_units.size(); m_compile_units.push_back(cu_sp); offset = cu_sp->GetNextCompileUnitOffset(); } ``` ? Also, shouldn't the error be propagated? https://reviews.llvm.org/D32167 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits