[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