[PATCH] D29892: ar: add llvm-dlltool support
Martell Malone via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 29 16:19:22 PDT 2017
martell added a comment.
In https://reviews.llvm.org/D29892#713612, @ruiu wrote:
> In https://reviews.llvm.org/D29892#713608, @martell wrote:
>
> > In https://reviews.llvm.org/D29892#713457, @ruiu wrote:
> >
> > > I want to see a patch to remove code from LLD as they should be checked in as a pair with this patch.
> >
> >
> > I can do that, my question on the parser remains to do this though because that is also mostly shared with lld bar a few tweaks?
>
>
> That is an unanswered question. You are copying this large code, and if you would fail to find a good way to remove the duplicated code from LLD in a follow-up patch, we'd have two duplicate parsers, which I want to avoid. You want to find a good way to avoid duplication.
I think you missed my inline comment and this got out of context
Makes sense.
I will have a look at using Expected or Error
For the Configuration it might be best to add to the parser and have that as part of llvm also because it can mostly be reused for llvm-lib
Have you any suggestions on where this would live?
I'm looking for an acceptable location to have the .def parser within the llvm code base.
Repository:
rL LLVM
https://reviews.llvm.org/D29892
More information about the llvm-commits
mailing list