[PATCH] D99055: [llvm-objcopy][NFC] remove processing of ELF specific options from common CopyConfig structure.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 22 01:45:52 PDT 2021


jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

> It also makes format of error mesages for --add-symbol option matching with format of error mesages for other options.

Could you do this in a separate patch upfront, please? As things stand, this patch is not NFC, because of this...! I have some concerns about the specific nature of the non-NFC part, which I'd like to comment on separately.

As for the main part of this change, I think this is essentially trying to redo D67139 <https://reviews.llvm.org/D67139>, which had a lot of back and forth discussion on it. Apart from anything else, the `*Objcopy.cpp` files are not where to put command-line parsing logic, which was an item from that review. Please go over the comments from D67139 <https://reviews.llvm.org/D67139>, and make sure to take the points raised in the comments on board. Once you have done that, feel free to propose a new solution (which could be similar to this, or something else entirely), if you feel it is important.


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