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

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

> FWIW, I think the current design generally looks reasonably good to me, but I'm open to other design ideas that satisfy the goals of both the new use-case and the existing behaviour. If an alternative looks simpler/more effective, please feel free to shout, though it might be easier to write it in a different patch so that we can compare and contrast competing approaches.

The current design is also OK from my point of view. The only thing is that it seems to me that using inheritance instead of encapsulation for CopyConfig structures would make usage more convenient/simple(https://reviews.llvm.org/D99055?id=334944).

> @avl, when I meant see a use-case, I meant literally see a patch that makes use of this refactoring in its current form. If a later patch can't be cleanly based on this patch, then it indicates a problem with the current design.

It would take a time to prepare such a patch. The fully ready use case - is some fully implemented tool, it requires a lot of other refactorings and changes. Anyway, would change https://reviews.llvm.org/D86539 to use refactoring from this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055