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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
