[PATCH] D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections.

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: lib/Object/Compressor.cpp:17
+namespace llvm {
+namespace object {
+
----------------
plotfi wrote:
> jakehehrlich wrote:
> > plotfi wrote:
> > > plotfi wrote:
> > > > jakehehrlich wrote:
> > > > > jhenderson wrote:
> > > > > > jakehehrlich wrote:
> > > > > > > This probably shouldn't be defined in object since that's a library namespace, maybe objcopy instead?
> > > > > > I nearly made the same mistake myself, but this is actually IN the Object library, so is in the correct namespace!
> > > > > Ohhhh. Yeah I think I want to put the kibosh on doing this in this change. For the purposes of llvm-objcopy and this change I don't think we want or need this. If this is truly needed at large then someone else should review it.
> > > > Yeah, this is in the object library which is the same place Decompressor is. 
> > > Alright, I will put the compressor library in a separate review diff. I do not want to write compression in llvm-objcopy because I see a lot of potential for refactoring redundant code all over llvm for this. 
> > > 
> > > So what exactly is your preference specifically?
> > It isn't up to me weather or not this goes into libObject. If you want to use it in this change then the final form of it that winds up in libObject may have use here, we'll see.
> Ah fair. How about I add the Compressor class to objcopy to start, and move it out in a another review / commit? 
You'll have to convince me that adding the Compressor class here is better than what I've proposed to get my blessing. I'll also accept what Alex and James accept on that matter even if it contradicts what I think.


Repository:
  rL LLVM

https://reviews.llvm.org/D49678