[PATCH] D99055: [llvm-objcopy][NFC] remove processing of ELF specific options from common CopyConfig structure.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 26 04:51:45 PDT 2021


avl updated this revision to Diff 333535.
avl added a comment.
Herald added subscribers: aheejin, sbc100.

Changed a patch to solve following problems:

      

1. Current CopyConfig has parsed and unparsed options at the same time. Thus, parsing functionality is located in different places. Instead, It would be good to parse options once and then have parsed options inside CopyConfig. This patch does all parsing job in the parse*() routines.
2. Current trunc implementation has an attempt to hide format-specific options. This patch makes it in a more general way: separate format-specific options using subclasses and allow to create the format-specific views at all options.

  So, this patch creates some interfaces representing common and format-specific options:

  CopyConfigBase CopyConfigELF CopyConfigCOFF CopyConfigMachO CopyConfigWasm

  And a class that allows to switch the view on the full options set:

  MultiFormatCopyConfig.

  I am a bit unsure whether it is finally good solution. From one side, It creates a good separation of concerns: clearly categorize options and separate them from other formats. From other side, the implementation looks a bit complicated.

  @jhenderson @alexshap @MaskRay What is your opinion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055

Files:
  llvm/tools/llvm-objcopy/CMakeLists.txt
  llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp
  llvm/tools/llvm-objcopy/COFF/COFFObjcopy.h
  llvm/tools/llvm-objcopy/COFF/CopyConfigCOFF.h
  llvm/tools/llvm-objcopy/CopyConfig.cpp
  llvm/tools/llvm-objcopy/CopyConfig.h
  llvm/tools/llvm-objcopy/CopyConfigBase.h
  llvm/tools/llvm-objcopy/CopyConfigFull.h
  llvm/tools/llvm-objcopy/ELF/CopyConfigELF.h
  llvm/tools/llvm-objcopy/ELF/ELFConfig.cpp
  llvm/tools/llvm-objcopy/ELF/ELFConfig.h
  llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
  llvm/tools/llvm-objcopy/ELF/ELFObjcopy.h
  llvm/tools/llvm-objcopy/ELF/Object.h
  llvm/tools/llvm-objcopy/MachO/CopyConfigMachO.h
  llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp
  llvm/tools/llvm-objcopy/MachO/MachOObjcopy.h
  llvm/tools/llvm-objcopy/MultiFormatCopyConfig.h
  llvm/tools/llvm-objcopy/ParsedConfig.cpp
  llvm/tools/llvm-objcopy/ParsedConfig.h
  llvm/tools/llvm-objcopy/llvm-objcopy.cpp
  llvm/tools/llvm-objcopy/llvm-objcopy.h
  llvm/tools/llvm-objcopy/wasm/CopyConfigWasm.h
  llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp
  llvm/tools/llvm-objcopy/wasm/WasmObjcopy.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D99055.333535.patch
Type: text/x-patch
Size: 69084 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210326/ca0e6a96/attachment.bin>


More information about the llvm-commits mailing list