[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:24
+  // forward.
+  extractor.emplace();
+
----------------
Don't we want to put an empty DWARFDataExtractor in here? Maybe this code should be:

```
const SectionList *section_list = module.GetSectionList();
  if (!section_list)
    return nullptr;

  auto section_sp = section_list->FindSectionByType(section_type, true);
  if (!section_sp)
    return nullptr;

  extractor.emplace();
  if (section_sp->GetSectionData(*extractor) == 0)
    extractor = llvm::None;
  return extractor.getPointer();
```


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:40
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+  return LoadOrGetSection(m_module, eSectionTypeDWARFDebugAranges,
----------------
Why do we care about "getOrLoadArangesData()"? What not just "getArangesData()"?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552
+  data.Clear();
+  m_obj_file->ReadSectionData(section_sp.get(), data);
 }
----------------
```
if (m_obj_file->ReadSectionData(section_sp.get(), data) == 0)
  data.Clear();
```


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

https://reviews.llvm.org/D59562