[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