ilya-biryukov added a comment. In D56841#1361492 <https://reviews.llvm.org/D56841#1361492>, @klimek wrote:
> (I know, I know, I'm not a big help) That's fine, mostly wanted to hear your opinion :-) > That said, if we can find a nice abstraction to pull out, I'm generally in > favor :) Eric suggested putting this into `lib/Tooling/ArgumentsAdjusters.cpp`, looks like a proper place for this kind of thing. > As a side note ArgumentsAdjusters 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. Yeah, running the frontend later would definitely outweigh this inefficiency, so we don't care. I do agree that's an unfortunate design, though. ================ Comment at: clangd/ClangdUnit.cpp:433 + // <arbitrary-argument> + if (I + 4 < E && CommandLine[I] == "-Xclang" && + (CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" || ---------------- kadircet wrote: > ilya-biryukov wrote: > > Maybe move this code into `ClangdServer::getCompileCommand` to keep **all** > > argument adjustments that we do in clangd in one place. > > We add `-resource-dir` there. > As mentioned in the change description, we also have a different compiler > invocation in `BackgroundIndex` which is not aware of > `ClangdServer::getCompileCommand` Ah, right :-( That's annoying. Does it also manually override `-resource-dir`? I'd move both into a single place, maybe this function is a better place to do so. 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
