[PATCH] D99055: [llvm-objcopy] Refactor CopyConfig structure.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 30 20:47:44 PDT 2021


dblaikie added a comment.

So the general point of this change is that previously all the options were in one struct which meant that, say, the MachO implementation could accidentally end up using an ELF option value, etc? And this way the implementations can accept the common options (the base class) and their implementation-specific options?

Any sense of whether that sort of cross-use was/is a significant source of bugs, or whether it'd be OK to just have chunks of the config specify "these are for ELF", "these are for MachO", etc (just in comments, or perhaps using ELF/MachO as a prefix, or having them in an ELF/MachO/etc subobject - while still having them all together/without the ability to create a common-and-MachO-view, common-and-ELF-view, etc) - I guess this came out of discussions in other reviews around the "it is necessary to minimize internal dependencies."? Might be good to have more of that context here to motivate/explain the benefits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055



More information about the llvm-commits mailing list