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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D99055#2644077 <https://reviews.llvm.org/D99055#2644077>, @avl wrote:

> addressed comments:
>
> Restored ELFConfig.h/cpp as a separate file keeping options parsing code.
> Left usage of parsing code of elf-specific options inside /ELF directory.
>
> D67139 <https://reviews.llvm.org/D67139> suggested parsing specific options lazily.
> i.e. To parse them only in case when they are necessary.
> But it places format-specific options in a common CopyConfig structure.
> This does not look good since all users of ELFConfig are inside of
> /ELF directory(Thus we do not need to make them visible to others).
> This patch hides elf-specific processing inside /ELF directory
> and parses format-specific options lazily.

Lazy parsing previously also had the advantage that the parsing only needed to happen once at most, even if there were multiple input files. It looks like this change causes the ELF specific config to be calculated once per input, which seems suboptimal? Aside from that, this looks like a general improvement to me. What do others think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055