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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 17 05:43:11 PST 2019


ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdUnit.cpp:429
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+    // According to https://clang.llvm.org/docs/ClangPlugins.html
----------------
ioeric wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > This doesn't seem to be clangd specific; clang-tidy seems to have the same issue. Could we share the filtering logic (e.g. in lib/Tooling/ArgumentsAdjusters.cpp)?
> > > We need a comment mentioning that we are filtering out the plugin options and why we're doing that here
> > +1 to the suggestion.
> > WDYT about landing it into clangd as a quick workaround and moving into tooling later?
> > The reason I think it might be better is because that would unblock usage of clangd in Chromium.
> > WDYT about landing it into clangd as a quick workaround and moving into tooling later?
> > The reason I think it might be better is because that would unblock usage of clangd in Chromium.
> I don't see why it would take much more effort to move the shared logic into tooling now? 
My approximation would be half an hour to finish this and land in clangd and 2-3 hours to land this in tooling, update clangd, clang-tidy, etc.
Not a big difference, though, up to Kadir to actually quantify how hard he thinks this would be.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841





More information about the cfe-commits mailing list