[PATCH] D49758: [clangd] allow clients to pass in compilationDatabaseChanges in the 'workspace/didChangeConfiguration' request

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Just a few nits left.



================
Comment at: clangd/ClangdLSPServer.cpp:422
+                        std::move(Entry.second.workingDirectory),
+                        llvm::sys::path::filename(File),
+                        std::move(Entry.second.compilationCommand),
----------------
Other usages of FileName in tooling::CompileCommand seem to set this either to an absolute or a relative-to-cwd path to the file for the command.
Maybe put the whole filepath there too? (even though I don't think we particularly rely on this anywhere in clangd)


================
Comment at: clangd/ClangdLSPServer.cpp:539
+    static_cast<InMemoryCompilationDb *>(CDB.get())->invalidate(File);
+  else if (CachingCDB)
+    CachingCDB->invalidate(File);
----------------
Other methods already assume it's non-null for directory-based CDB, so maybe remove the check for it being non-null here or replace the check with an assert?


================
Comment at: test/clangd/did-change-configuration-params.test:43
+#
+# ERR: Updating file /clangd-test/foo.c with command [/clangd-test2] clang -c foo.c -Wall -Werror
+# Don't reparse the second file:
----------------
Matching the stderr output seems fragile, but that's the only way to inspect the actual compile command that's used, so that's probably fine here.
However, the paths on windows will probably differ (notice the regexp matches for file uris), so we might need to do the regexp match here as well, e.g.`Updating file {{.*}}foo.c with command ...` to keep the Windows buildbots happy.


https://reviews.llvm.org/D49758