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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/Object.h:17
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/MC/MCTargetOptions.h"
 #include "llvm/MC/StringTableBuilder.h"
----------------
jhenderson wrote:
> jhenderson wrote:
> > plotfi wrote:
> > > jhenderson wrote:
> > > > You can get rid of this from the header by forward-declaring the DebugCompressionType enum.
> > > You can not. Usage of DebugCompressionType::GNU etc makes this not possible unless you want to copy the enum class (which I would like not to).
> > You aren't using them in the header file, as far as I can see. Move the include into the .cpp if it is needed there. Same goes for Compression.h.
> You should still forward declare the enum in the header and include MCTargetOptions.h in the cpp file.
@jhenderson - khm, if we forward declare the enum the size of the enum is unknown (if the definition is not available), consequently, every single .cpp which includes Object.h will have to include MCTargetOptions.h. Just curious - what are the benefits of this / are there any pointers to the llvm style guide or any other docs suggesting this ? that would look kinda broken (one header (Object.h) implicitly depends on another one and can not be included without causing errors into an empty .cpp). 


Repository:
  rL LLVM

https://reviews.llvm.org/D49678