[PATCH] D37150: [clangd] Command line arg to specify compile_commands.json path
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 21 19:14:19 PDT 2017
ilya-biryukov added a comment.
Could you please run `clang-format` on every submission?
================
Comment at: clangd/GlobalCompilationDatabase.cpp:98
+ {
+ CompileCommandsDir = "/";
+ return tryLoadDatabaseFromPath(CompileCommandsDir.getValue());
----------------
Is this some kind of accidental change? Why do we need to assign `"/"` to `CompileCommandsDir`?
================
Comment at: clangd/tool/ClangdMain.cpp:20
#include <thread>
+#include <unistd.h>
----------------
We certainly don't need that include.
================
Comment at: clangd/tool/ClangdMain.cpp:79
+ CompileCommandsDirPath = llvm::None;
+ else
+ {
----------------
This should be something like:
```
if (CompileCommandsDir.empty()) {
//...
CompileCommandsDirPath = llvm::None;
} else if (!is_absolute(...)) {
//....
CompileCommandsDirPath = llvm::None;
} else if (!exists(...)) {
// ....
CompileCommandsDirPath = llvm::None;
} else {
CompileCommandsDirPath = CompileCommandsDir;
}
```
- It will have less nesting, therefore making code more readable.
- It will fix an error in the current implementation. (Currently, `exists` check will run on an empty string if `-compile-commands-dir` is not an absolute path).
https://reviews.llvm.org/D37150
More information about the cfe-commits
mailing list