[PATCH] D63185: [llvm-objcopy] [WIP] Librarify llvm-objcopy

Thu Jun 13 02:02:40 PDT 2019

jhenderson added a comment.

Okay, I think I see what you're getting at now. By creating a copy, you don't need to have the OriginalX fields. I think this makes sense now, okay.

> (although I cannot find his response on Google groups so I'm not quite sure how to link it)

There's an archive for the email threads which I usually use to link to past llvm-dev emails: http://lists.llvm.org/pipermail/llvm-dev/

> FWIW Jake had also said that OriginalXXX were only used in objcopy and wouldn't make sense to prioritize for a library, but I thought this might be a clever way to incorporate that.

I think you'll find once you start trying to write file layout code that you'll need access to the "Original" values at various points. That's why they exist in llvm-objcopy. I don't think we should leave laying the file out to the user (or at least, I think we should have a function that they can choose to call to lay it out, if they want to follow the default layout routine), so you'll need the data.

> Do you think there should be no common base class like there is in the current llvm-objcopy code? I would say that libObject has quite a bit in the ObjectFile (and in SymbolicFile and Binary) class and leaves only the truly file dependent routines in their respective classes.

I'll admit that I haven't thought too much about the differences between the file formats, and how libObject is designed. Thinking about it a bit more, I guess it might be a nice design to have effectively a mutable mirror that sits on top of libObject, with a parallel hierarchy like you have started. I don't have strong feelings either way though. I guess I want to know how you envisage a regular user using the library as a whole, because I suspect we might have slightly different visions here that could explain some of the differences.

> Is the alternative you are suggesting how the current code is? You had said that the current code is not a library in LLVM terms, what is the biggest factor making this true?

I assume by current code you are referring to the existing llvm-objcopy code. At the moment, the code is in a location that is not intended to be used outside the llvm-objcopy tree (llvm-strip is similar enough that it is effectively just a clone of llvm-objcopy). A library needs to live in llvm/include and llvm/lib, so that different tools can link against it. In my mind, the only part that should really be outside the library is the driver components in llvm-objcopy.cpp (and maybe CopyConfig.cpp, but I'm not convinced on that point). However, that being said, your design sounds like it might provide a bit more flexibility for library users in the future, so please keep going with it.

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

https://reviews.llvm.org/D63185