[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