[PATCH] D48413: [llvm-objcopy] Add initial support for static libraries

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/Object.h:130
+
+class Buffer {
+  StringRef Name;
----------------
alexshap wrote:
> jhenderson wrote:
> > I'm probably missing something, but what's wrong with the existing buffer classes in llvm? Would it make more sense to extend them in the general case, rather than a specific reimplementation here?
> > 
> > It would be nice to see some comments describing what the interface functions do.
> it's not a reimplementation per se. FileOutputBuffer and WritableMemoryBuffer share a big portion of the interface but don't have a common base class. They are used in many places across  llvm - so i didn't want to change them for now. I've tried several approaches, I liked this one for several reasons: (imo) it's cleaner than the other options, the concerns are nicely factored out, the writers don't depend on the implementation of the buffers (but depend on the interface). 
i'd prefer to add a TODO here to refactor the buffer classes in llvm and then get rid of these, rather than blocking this patch or doing it here. 


https://reviews.llvm.org/D48413