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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 20 01:43:17 PDT 2021


jhenderson added a comment.

In D99055#2699528 <https://reviews.llvm.org/D99055#2699528>, @avl wrote:

>> What do you think?
>
> This solution has some concerns caused by lazy options parsing.
> I do not understand whether/why we need lazy parsing.
> Thus, depending on the reason for lazy parsing, these concerns might be acceptable price or might be not. To support lazy parsing it is necessary 
> to keep unparsed data, move some parsing functionality into parseELFConfig(). 
> In case it would be necessary to create another tool(llvm-another-tool) then
> we need to implement a child of ConfigManager and implement another version of 
> parseELFConfig(). All this is doable but looks like unnecessary complications.
> If it would be not necessary to support lazy parsing then the solution would look simpler. 
> Do we need to support lazy parsing?

I've not checked, but if memory serves me correctly, there are some options (--add-symbol, I think, at least) that are parsed differently depending on the file format. As such, we can't parse them upfront - if we did, one of the two parse*Config functions would fail. We'd only want that to happen if a user was providing both file formats and the option. Additionally, we don't want to warn about unsupported options for specific file formats, if we don't use those file formats.


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