This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] [COFF] Remove pointless Reader/Writer base classes. NFC.
ClosedPublic

Authored by mstorsjo on Jan 8 2019, 3:26 AM.

Details

Summary

These were copied as part of the original design from the ELF backend, but aren't necessary at the moment.

In the original review of the initial COFF backend, I tried to get an opionion on whether to keep them or not, but I guess it drowned in the rest of that review. @alexshap pointed out in D55881 that they could be removed.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Jan 8 2019, 3:26 AM
jhenderson accepted this revision.Jan 8 2019, 3:47 AM

LGTM. I have no issue with this, but I'd like @jakehehrlich to give his opinion as essentially it's a design choice as to how much the COFF structure of llvm-objcopy should match the ELF structure.

This revision is now accepted and ready to land.Jan 8 2019, 3:47 AM
rupprecht accepted this revision.Jan 11 2019, 1:42 PM

I think there's a chance we'll end up reintroducing this in some form, e.g. to support something like "-I binary -O <coff format>" (or vice versa), but it's usually a bad idea to leave speculative code lying around, so LGTM.

This revision was automatically updated to reflect the committed changes.