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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/CMakeLists.txt:32-47
+  ObjCopy/ObjCopy.cpp
+  ObjCopy/COFF/COFFObjcopy.cpp
+  ObjCopy/COFF/Object.cpp
+  ObjCopy/COFF/Reader.cpp
+  ObjCopy/COFF/Writer.cpp
+  ObjCopy/ELF/ELFObjcopy.cpp
+  ObjCopy/ELF/Object.cpp
----------------
avl wrote:
> jhenderson wrote:
> > avl wrote:
> > > jhenderson wrote:
> > > > I thought ObjCopy was going to be its own distinct library, not a part of the Object library? It would make more sense to me for it to be separate, as it's a fairly distinct piece of functionality.
> > > It was directly requested to implement it as part of Object library during the previous review iteration:
> > > 
> > > https://reviews.llvm.org/D88827#2344900
> > Right, there was some opposition from @sbc100 though. I think we need to hear their thoughts on that.
> > 
> > If we do end up putting it in libObject, should the namespace be changed? Not sure either way.
> Do you mean:
> 
> objcopy->object?
> objcopy->objcopybase?
> 
> I think it is fine to have objcopy(like in this patch). Changing objcopy->object creates some names clashes. Thus we need to have other namespace than object for the objcopy part. Having objcopy inside Object library and inside llvm-objcopy tool seems to be no problem.
> 
I was referring to the first of those (using `object` instead of `objcopy`). If using the same namespace would be a problem, I don't see any issue with leaving as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88827