[PATCH] D63194: [clangd] Link in target infos and pass target and mode while invoking driver

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks, this looks very good!
Leaving a few nitpicky comments, but nothing important really.

Two more general NITs:

- could we update the title and description of this revision to mirror that it focuses on code in tooling rather than in clangd?
- maybe land the clangd parts into a separate change for nicer VCS history?



================
Comment at: clang/include/clang/Tooling/CompilationDatabase.h:220
+std::unique_ptr<CompilationDatabase>
+getTargetAndModeAdderDatabase(std::unique_ptr<CompilationDatabase> Base);
+
----------------
NIT: in the spirit of a previous name, we could shorten the name `inferTargetAndDriverMode(…)`


================
Comment at: clang/lib/Tooling/CMakeLists.txt:25
   StandaloneExecution.cpp
+  TargetAndModeAdderDatabase.cpp
   Tooling.cpp
----------------
NIT: `GuessTargetAndDriverModeCompilationDatabase` would align better with the names of other files that define compilation databases.

It looks a little long, though, maybe there's a better word to capture `TargetAndMode`


================
Comment at: clang/lib/Tooling/TargetAndModeAdderDatabase.cpp:39
+  std::vector<CompileCommand>
+  addTargetAndMode(std::vector<CompileCommand> &&Cmds) const {
+    for (auto &Cmd : Cmds) {
----------------
NIT: maybe accept by value to simplify the code?
A cost of an extra move constructor here is negligible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63194