This is an archive of the discontinued LLVM Phabricator instance.

Share /machine: handling code with llvm-cvtres too
ClosedPublic

Authored by thakis on Jun 10 2019, 8:40 PM.

Details

Summary

r363016 let lld-link and llvm-lib share the /machine: parsing code.
This lets llvm-cvtres share it as well.

Making llvm-cvtres depend on llvm-lib seemed a bit strange (it doesn't
need llvm-lib's dependencies on BinaryFormat and BitReader) and I
couldn't find a good place to put this code. Since it's just a few
lines, put it in lib/Object for now.

Diff Detail

Event Timeline

thakis created this revision.Jun 10 2019, 8:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 8:40 PM
ruiu added a subscriber: ruiu.Jun 11 2019, 5:32 AM

I can see why you created a new library, but it still look a bit odd that the actual library code is less than 40 lines. Do you think adding this to Object make sense? This is not really Object-related, but I guess adding a small amount of code there wouldn't harm anyone.

Ok, unless someone else objects to it I'll upload a modified version that puts the code in lib/Object instead tomorrow.

thakis updated this revision to Diff 204112.Jun 11 2019, 10:55 AM
thakis edited the summary of this revision. (Show Details)

now in lib/object

thakis updated this revision to Diff 204117.Jun 11 2019, 11:09 AM
thakis updated this revision to Diff 204118.
ruiu accepted this revision.Jun 11 2019, 8:43 PM

LGTM

This revision is now accepted and ready to land.Jun 11 2019, 8:43 PM
This revision was automatically updated to reflect the committed changes.