[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