[PATCH] D88827: [llvm-objcopy][NFC] Move core implementation of llvm-objcopy into separate library.

Thu Oct 22 14:03:45 PDT 2020

sbc100 added a comment.

In D88827#2346567 <https://reviews.llvm.org/D88827#2346567>, @jhenderson wrote:

> In D88827#2344916 <https://reviews.llvm.org/D88827#2344916>, @sbc100 wrote:
>
>> In D88827#2344900 <https://reviews.llvm.org/D88827#2344900>, @echristo wrote:
>>
>>> I really do think this needs to be under object. It can be separate in there as lib/Object/Copy if you want, but I don't think it should be a parallel directory. Hopefully the updates for the move aren't overly onerous here.
>>>
>>> -eric
>>
>> Won't that mean that any tool that just wants to read in object files (e.g. lld) will need to build this extra code for no reason?  Not a huge deal I guess (especially for lld which due to LTO pulls in all of llvm!) but I imagine there are many tools that just want to read (and not modify) objects.
>
> That will depend on the archiver/linker behaviour, I expect, but at least for standard ld.lld behaviour (I can't speak for others), only the objects that actually are needed are pulled from the archive. Therefore, for example, if LLD didn't reference anything in the objcopy codebase, it wouldn't actually use those archive members, and therefore it wouldn't be added to the tools code. Even if it did, linker options like --gc-sections or equivalent likely would cause the unused code to be removed at link time, if the tool is built appropriately.

You are right, the size of resulting tools won't be increased.   I was referring to the fact that such a tool would transitively depend on more sources and would take longer to compile, and would become dirty more often on average after a `git pull`.   For example, its good that one can build llvm-objdump without compiling the whole of llvm.

> @alexshap's point about other files in the Object library allowing writing is persuasive to me. I think an Objcopy sub-directory would make sense though, to avoid the confusion of having both an ObjectFile and Object class/header file etc in Object. This is assuming of course we don't want to jump into refactoring the two classes so that they become one. I suspect that way might prove tricky to get right.
>
> I'm somewhat ambivalent as to whether we move then refactor or refactor then move. If people feel that the latter approach is the right one, then so be it.
>
> Regarding the naming of Object.h, that's fine, leave it as-is. I suspect with some small CMake changes, it should be possible to get the files to appear in a corresponding "solution folder" within the VS IDE, which would help disambiguate things.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88827/new/

https://reviews.llvm.org/D88827