This is an archive of the discontinued LLVM Phabricator instance.

[DWARFYAML] Make the debug_abbrev_offset field optional.
ClosedPublic

Authored by Higuoxing on Aug 26 2020, 5:22 AM.

Details

Summary

This patch helps make the debug_abbrev_offset field optional. We don't
need to calculate the value of this field in the future.

Diff Detail

Event Timeline

Higuoxing created this revision.Aug 26 2020, 5:22 AM
Higuoxing requested review of this revision.Aug 26 2020, 5:22 AM

For some reason, I didn't get any email notification about this review. Could you let me know if there are any other patches you want me to review, please?

llvm/lib/ObjectYAML/DWARFEmitter.cpp
99

We should probably not be using report_fatal_error here. Either an Expected needs returning up the stack, or this should be an assertion, depending on whether a user input could reach this case or not.

449–455

I might be misremembering the llvm-dev conversation, but I think the style is if any level of an if statement requires braces, they all do (i.e. please add them to the inner if).

454

Please add a comment saying why it's okay to consumeError here rather than report it.

Higuoxing updated this revision to Diff 288231.Aug 27 2020, 1:43 AM
Higuoxing marked 3 inline comments as done.

Address comments.

For some reason, I didn't get any email notification about this review. Could you let me know if there are any other patches you want me to review, please?

All of my uploaded patches are reviewed, thanks!

jhenderson accepted this revision.Aug 27 2020, 2:04 AM

LGTM, thanks.

llvm/lib/ObjectYAML/DWARFEmitter.cpp
456
This revision is now accepted and ready to land.Aug 27 2020, 2:04 AM