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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a subscriber: jhenderson.
dblaikie added a comment.

In D99055#2655081 <https://reviews.llvm.org/D99055#2655081>, @jhenderson wrote:

> Thanks for the update. I do have some concerns about the multiple inheritance stuff going on, if I'm honest, as usually this is a design smell

+1 I can take a look more closely to try to tease out whether there's some alternative that might be nicer, but maybe @rupprecht or @maskray might have more context here already. (at a glance the use of virtual inheritance here seems plausible - since these are just structs - if all the config property access went through virtual functions then it could probably be done without virtual inheritance, but with virtual functions and maybe some CRTP mix-in kind of implementations, but that may not be any better - but I wouldn't mind a few other eyes on this, maybe there's some other ways forward here)

> I do still think you should normalise the error messages in a separate prerequisite change. Small changes are good, especially if it helps make something else a pure refactor.

Sounds pretty reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055