sammccall added a comment.

In D56841#1361395 <https://reviews.llvm.org/D56841#1361395>, @kadircet wrote:

> It seems like the most relevant place in tooling is ArgumentsAdjuster as 
> @ioeric pointed out. Happy to move it to there if @sammccall and @klimek 
> agrees as well.
>
> As a side note `ArgumentsAdjuster`s unfortunately causes a copy of the 
> original command line arguments. Not sure how important this factor is 
> compared to spinning up a compiler instance, just wanted to point it out.


I do think this should be a standard arguments adjuster available in Tooling.
This is a pattern that's likely to recur (clang-tidy will need the same fix to 
work with chromium). The fix is small but fiddly enough that it might warrant 
some improvement over time - I think we want to avoid people cloning their own 
copies of the fix.
The ArgumentsAdjuster interface is a little unfortunate regarding the copy, but 
I don't think this is significant in practice.

Once verifying this fixes the issue, we should consider requesting the tooling 
and clangd patches be cherrypicked to the LLVM-8 branch.
(And possibly do the same for clang-tidy)


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56841/new/

https://reviews.llvm.org/D56841



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to