[PATCH] D106669: [clangd] Unify compiler invocation creation

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/Compiler.h:64
 
+/// Clears \p CI from options that are not supported by clangd, like codegen or
+/// plugins.
----------------
Comment is good, but call this disableUnsupportedOptions?
(We're in namespace clangd already)


================
Comment at: clang-tools-extra/clangd/Compiler.h:64
 
+/// Clears \p CI from options that are not supported by clangd, like codegen or
+/// plugins.
----------------
sammccall wrote:
> Comment is good, but call this disableUnsupportedOptions?
> (We're in namespace clangd already)
This should also probably have a comment about being paired with CommandMangler, as that provides similar functionality that just happens to be best expressed on the command instead (e.g. stripping --save-temps) 


================
Comment at: clang-tools-extra/clangd/test/indexer.test:3
+# RUN: touch %t.cpp
+# RUN: clangd-indexer %t.cpp -- -Xclang -verify --save-temps -- 2>&1 | FileCheck %s
+# CHECK-NOT: error: no expected directives found: consider use of 'expected-no-diagnostics'
----------------
Add a comment (or rename test file) saying what functionality this is trying to test? it's not obvious


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106669