This is an archive of the discontinued LLVM Phabricator instance.

llvm-objdump: move SourcePrinter to shared file
ClosedPublic

Authored by t.p.northover on Apr 14 2021, 5:46 AM.

Details

Summary

Interleaved source printing during disassembly is a useful feature, but all the classes for it live in llvm-objdump.cpp and can't be used by the (weirdly) separate MachODumper.cpp. This patch pretty mechanically moves the needed bits (and command-line options) into a shareable header and adds the needed hooks for MachO.

Diff Detail

Event Timeline

t.p.northover created this revision.Apr 14 2021, 5:46 AM
t.p.northover requested review of this revision.Apr 14 2021, 5:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2021, 5:46 AM

Looks like this is causing test failures? Maybe you could break it down into a couple of patches, with the first being the simple moving of code, and later patches adding the new functionality for MachO?

Moving the code is a good time to address any formatting issues highlighted by clang-tidy/clang-format too.

Thanks, this should now be just the NFC refactoring, with most clang-tidy/clang-format suggestions applied (I ignored the DWARF one for obvious reasons).

I'm not sure what's going on with those test failures, it works on my local Linux machine. But now that this is non-MachO that's a problem for another time.

jhenderson accepted this revision.Apr 20 2021, 5:30 AM

LGTM, with nits.

llvm/tools/llvm-objdump/SourcePrinter.cpp
482

Nit: add a new line at EOF.

llvm/tools/llvm-objdump/SourcePrinter.h
164

Nit: add a new line at EOF

This revision is now accepted and ready to land.Apr 20 2021, 5:30 AM
t.p.northover closed this revision.Apr 23 2021, 2:22 AM

Thanks. Committed as c623945d707c with the new lines.