This is an archive of the discontinued LLVM Phabricator instance.

Remove `SymbolFileDWARF *` when there is already `DWARFUnit *`
ClosedPublic

Authored by jankratochvil on May 16 2019, 7:36 AM.

Details

Summary

In D61502#1503247 @clayborg suggested that SymbolFileDWARF *dwarf2Data is really redundant in all the calls with also having DWARFUnit *cu. So remove it. I see no regressions.

There are some nullptr checks, I changed those needed from dwarf2Data == nullptr to cu == nullptr but then who knows if they can happen.

Diff Detail

Repository
rL LLVM

Event Timeline

jankratochvil created this revision.May 16 2019, 7:36 AM
jankratochvil marked 3 inline comments as done.May 16 2019, 7:37 AM
jankratochvil added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
380 ↗(On Diff #199828)

Here is a NULL check.

1055 ↗(On Diff #199828)

Here is a NULL check.

1086 ↗(On Diff #199828)

Here is a NULL check.

Seems like a nice cleanup. Given that code code already assumed that the DWARFUnit can't be null, I'd just delete the null checks..

Seems like a nice cleanup. Given that code code already assumed that the DWARFUnit can't be null, I'd just delete the null checks..

I have rechecked it now and 2 of the 3 tests are valid, it can be NULL, callers are:

DWARFDebugInfoEntry::DumpAttribute():
  case DW_AT_abstract_origin:
  case DW_AT_specification: {
    DWARFDIE abstract_die = form_value.Reference();
    GetName(abstract_die.GetCU(), abstract_die.GetOffset(), s);
  case DW_AT_type: {
    DWARFDIE type_die = form_value.Reference();
    AppendTypeName(type_die.GetCU(), type_die.GetOffset(), s);

Sorry for not verifying it myself first.

clayborg accepted this revision.May 21 2019, 9:53 AM
This revision is now accepted and ready to land.May 21 2019, 9:53 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2019, 10:36 AM