[PATCH] D56841: [clangd] Filter out plugin related flags.

[Ilya Biryukov via Phabricator via cfe-commits]

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