[PATCH] D116556: [llvm-objcopy] Implement the PE-COFF specific --subsystem option

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 4 14:38:50 PST 2022


mstorsjo added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ConfigManager.cpp:598
       !Common.SymbolsToAdd.empty()) {
     return createStringError(llvm::errc::invalid_argument,
                              "option not supported by llvm-objcopy for MachO");
----------------
On the topic of testing diagnostics... Here (in the `get*Config()` format specific methods) we do error out if a common option has been set, that isn't implemented for that format. But what if options in one of the different format specific structs have been set, e.g. an option set in `ELF`, but we end up calling `getMachOConfig()` - then we don't error out at all.

Should we add a separate `bool ELFOptionsSet` (and similar for all other formats) which we could check for in the `get*Config()` methods, so we error out cleanly in the same way, if format specific options are set for the wrong format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116556



More information about the llvm-commits mailing list