clayborg added a comment.
See inline comments. Nice patch overall though, real close.
================
Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:18
if (form_value.IsValid()) {
const DWARFUnit *dwarf_cu = form_value.GetCompileUnit();
if (dwarf_cu) {
----------------
We should maybe rename DWARFFormValue::GetCompileUnit() to
DWARFFormValue::GetUnit()? Also we might use "unit" instead of "dwarf_cu" here?
================
Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:19
struct DIERef {
+ enum DebugSection : uint8_t { InfoSection, TypesSection };
+
----------------
Use enum class?
```
enum class Section : uint8_t { DebugInfo, DebugTypes };
```
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:141
+ uint32_t idx = FindUnitIndex(section, cu_offset);
DWARFUnit *result = GetUnitAtIndex(idx);
if (result && result->GetOffset() != cu_offset) {
----------------
Does the "GetUnitAtIndex" call need to take "section" to get the right unit?
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:162
+ uint32_t idx = FindUnitIndex(section, die_offset);
DWARFUnit *result = GetUnitAtIndex(idx);
if (result && !result->ContainsDIEOffset(die_offset))
----------------
Does the "GetUnitAtIndex" call need to take "section" to get the right unit?
================
Comment at: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h:51-52
struct DIEInfo {
dw_offset_t cu_offset;
dw_offset_t offset; // The DIE offset
dw_tag_t tag;
----------------
Should we use a DIERef here instead of these two members?
================
Comment at: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h:61
+ explicit operator DIERef() const {
+ return {DIERef::InfoSection, cu_offset, offset};
+ }
----------------
just return the DIERef member variable if we switch according to above inline
comment
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61908/new/
https://reviews.llvm.org/D61908
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
