[PATCH] D128688: [llvm-objcopy] Remove support for legacy .zdebug sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/ObjCopy/ELF/ELFObject.cpp:522
   uint8_t *Buf = reinterpret_cast<uint8_t *>(Out.getBufferStart()) + Sec.Offset;
   if (Sec.CompressionType == DebugCompressionType::None) {
     std::copy(Sec.OriginalData.begin(), Sec.OriginalData.end(), Buf);
----------------
jhenderson wrote:
> MaskRay wrote:
> > jhenderson wrote:
> > > I wonder if this should be a switch statement, so that we can benefit from compiler diagnostics if not all `DebugCompressionType` values are handled. Thoughts?
> > I placed `assert(Sec.CompressionType == DebugCompressionType::Z);` below. When zstd is added, the assert will fire :)
> My thinking was that it would turn the runtime failure into a potential build-time one, which improves developer experience. We'd still want an llvm_unreachable or similar after the switch of course.
The build-time warning/error idea LG. I switched to a `switch` statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128688