[PATCH] D99055: [llvm-objcopy] Refactor CopyConfig structure.
Alexey Lapshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 26 13:33:07 PDT 2021
avl added inline comments.
================
Comment at: llvm/tools/llvm-objcopy/ConfigManager.h:26
+// format-specific options.
+struct ConfigManager : public MultiFormatConfig {
+ virtual ~ConfigManager() {}
----------------
dblaikie wrote:
> Why is there an inheritance relationship here - if there's only one class that implements MultiFormatConfig - why isn't ConfigManager a non-virtual API?
MultiFormatConfig interface is used when passed to routine working with archives:
```
Expected<std::vector<NewArchiveMember>>
createNewArchiveMembers(const MultiFormatConfig &Config,
const object::Archive &Ar);
```
It would be bad to pass whole ConfigManager here since ConfigManager has elements not used by createNewArchiveMembers. Additionally ConfigManager is a llvm-objcopy specific thing. For some another tool(llvm-another-tool) there might be another implementation of config manager, but it needs to be presented as a universal interface for createNewArchiveMembers. MultiFormatConfig is a such interface.
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