This is an archive of the discontinued LLVM Phabricator instance.

Update DwarfDebugInfoEntry to use llvm::Error and llvm::Expected
AcceptedPublic

Authored by zturner on Mar 15 2019, 1:12 PM.

Details

Summary

This hits the next batch of classes and adds better error handling and recovery to them. After this, the only remaining case is the line table stuff, at which point we should be able to get all logging out of the low level DWARF parsing entirely.

Diff Detail

Event Timeline

zturner created this revision.Mar 15 2019, 1:12 PM
aprantl added inline comments.Mar 15 2019, 1:28 PM
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
218

Should this be a lldb_assert() followed by return make_error instead?

1170

Instead of passing a pointer and asserting, wouldn't it be better to pass a SymbolFileDWARF &?

zturner marked 3 inline comments as done.Mar 15 2019, 1:35 PM
zturner added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
218

I think it's reasonable to treat this as an internal consistency check, where the pre-condition of this function is "offset must be a valid offset". Similar to indexing an array out of bounds, your operator[] implementation would assert that the index you passed is within range.

1170

Yes, but I was trying to keep the change set minimal, and doing so would propagate that change many levels up the callstack until we reach SymbolFileDwarf, at which point we would change all calls to pass *this instead of this. That's a reasonable change, but should probably be done separately to keep logically separate changes separate.

Note that, long term, we just won't even pass a SymbolFileDWARF to this function at all, because if the point is to decouple the high and low level interfaces, then the low-level interface can't know about the high level interface.

aprantl accepted this revision.Mar 18 2019, 9:40 AM
This revision is now accepted and ready to land.Mar 18 2019, 9:40 AM
labath resigned from this revision.Aug 9 2019, 1:59 AM