[Lldb-commits] [PATCH] D58330: 01/03: new SectionPart for Section subranges (for effective .debug_types concatenation)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/include/lldb/Symbol/ObjectFile.h:58
+private:
+  Section *const m_section;
+  // It should match: llvm::DWARFUnitIndex::Entry::SectionContribution
----------------
Should this be a SectionSP? If not, can we use a reference? 


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:551
   const SectionList *section_list = module_sp->GetSectionList();
   if (section_list) {
+    Section *section = section_list->FindSectionByType(sect_type, true).get();
----------------
Inver the case and make this an early return?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp:34
   if (section_list) {
-    SectionSP section_sp(section_list->FindSectionByType(sect_type, true));
-    if (section_sp) {
-      // See if we memory mapped the DWARF segment?
-      if (m_dwarf_data.GetByteSize()) {
-        data.SetData(m_dwarf_data, section_sp->GetOffset(),
-                     section_sp->GetFileSize());
-        return;
-      }
-
-      if (m_obj_file->ReadSectionData(section_sp.get(), data) != 0)
-        return;
-
-      data.Clear();
-    }
+    Section *section = section_list->FindSectionByType(sect_type, true).get();
+    if (section)
----------------
```
if (SectionSP section = section_list->FindSectionByType(sect_type, true)) 
  return SectionPart(section.get()); 
```
Would be a lot more readable imho.



================
Comment at: lldb/source/Symbol/ObjectFile.cpp:753
+    : m_section(section), m_offset(0), m_length(section->GetFileSize()) {
+  assert(m_section);
+}
----------------
Shouldn't these be `lldbassert`s?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D58330